labmlai
Fix enum support
In newer rails version brakeman does not recognize enums as safe.
:yellow_circle: Please give this pull request extra attention during review.
This pull request reveals multiple potential SQL injection vulnerabilities in a Rails application, specifically in the Group model and test files, where direct string interpolation in SQL queries could allow malicious SQL code injection without proper sanitization techniques.
:yellow_circle: Potential SQL Injection in test/apps/rails7/app/models/group.rb
| Vulnerability | Potential SQL Injection |
|---|---|
| Description | The code contains multiple potential SQL injection vulnerabilities: 1. In uuid_in_sql() method, the User.uuid is directly interpolated into the SQL query without sanitization, which could allow an attacker to inject malicious SQL code. 2. In date_in_sql() method, the date is directly interpolated into the SQL query using Arel.sql(), which could potentially be manipulated. 3. In fetch_constant_hash_value() method, while the role is fetched from a predefined hash, the role value is still directly interpolated into the SQL query using Arel.sql(), which is unsafe. 4. In use_simple_method(), the result of Group.simple_method() is directly interpolated into the SQL query without sanitization. Although ar_sanitize_sql_like() uses ActiveRecord:Base.sanitize_sql_like() which provides some protection, the other methods demonstrate direct string interpolation in SQL queries, which is a classic SQL injection vulnerability. These methods do not use parameterized queries or prepared statements, leaving them open to potential SQL injection attacks. |
https://github.com/presidentbeef/brakeman/blob/415731315e6fef194ebdb078c1428749f28ca9da/test/apps/rails7/app/models/group.rb#L1-L37
:yellow_circle: Potential SQL Injection in test/tests/rails7.rb
| Vulnerability | Potential SQL Injection |
|---|---|
| Description | The code snippet contains two test cases related to SQL injection scenarios. The first test case (test_sql_injection_sanitize_sql_like) demonstrates a potential SQL injection vulnerability when using Arel.sql() with ActiveRecord:Base.sanitize_sql_like(). While sanitize_sql_like() provides some protection, it does not completely prevent SQL injection in all contexts. The second test case (test_sql_injection_enum) shows a scenario involving enum values being used in a SQL query, which could potentially introduce SQL injection risks if not handled carefully. The presence of these test cases suggests that the code is checking for potential SQL injection vulnerabilities, indicating an awareness of the risks associated with unsanitized user input in database queries. |
https://github.com/presidentbeef/brakeman/blob/415731315e6fef194ebdb078c1428749f28ca9da/test/tests/rails7.rb#L462-L492
All finding details can be found in the DryRun Security Dashboard.
![dryrunsecurity[bot] avatar](https://avatars.githubusercontent.com/in/377039?v=4 "Published by a pub.dev verified publisher"){kind=small}
Brakeman still needs to support the older API for enums.
I'm not sure why you moved the Gemfile for the Rails 7 test app, there shouldn't be any need to touch it.
Brakeman still needs to support the older API for enums.
I'm not sure why you moved the Gemfile for the Rails 7 test app, there shouldn't be any need to touch it.
I wasn’t really sure how the whole thing with the tests works. Then I had a look at it and thought Rails 7 is different compared to the other apps. Anyway, I’ve now just added the test cases from Rails 6.
I think the new code isn’t being tested. When I set a debugger in the Rails 8 app, it still shows the old behavior. I’m not sure how this is supposed to work. In my app, it works fine.
Could it be that the to_test.rb is outdated as well? I was wondering — isn’t it possible to generate the test files automatically?
No need to add duplicate tests to different test apps.
You've only added new instances of the old-style API, so that's why the new code is not being exercised.
to_test.rb simply generates tests based on the warnings emitted. So to test a fixed false positive, what I do is add the false positive to the test app and run to_test.rb without the fix. That generates an assert_warning .... That can then be changed to assert_no_warning - which will fail at first. Then add in your fix and it should pass.