presidentbeef / brakeman

A static analysis security vulnerability scanner for Ruby on Rails applications
https://brakemanscanner.org/
Other
7.02k stars 732 forks source link

UnscopedFind for `find_by!` #1859

Closed presidentbeef closed 4 months ago

presidentbeef commented 4 months ago

Partially fixes #1786

dryrunsecurity[bot] commented 4 months ago

DryRun Security Summary

The provided code changes focus on improving the security of Ruby on Rails applications by enhancing the capabilities of the Brakeman security scanner and strengthening the test coverage for security-related issues, such as unscoped queries, SQL injection, cross-site scripting (XSS), and command injection.

Expand for full summary
**Summary:** The provided code changes cover several security-related improvements to the Brakeman security scanner for Ruby on Rails applications, as well as updates to the test suite for a Rails 4 application. The key security-focused changes include: 1. Expansion of the Brakeman check to cover additional ActiveRecord query methods, such as `find_by` and `find_by!`, to improve the detection of unscoped queries that can lead to security vulnerabilities. 2. Enhancements to the Brakeman check to handle optional `belongs_to` associations, which can also result in unscoped queries. 3. Improvements to the handling of potentially missing records in the `UsersController` by using the `find_by!` method, which raises an exception instead of returning a `nil` value, leading to more robust error handling. 4. Additions to the Rails 4 test suite, including a new test case for unscoped calls to the `Email#find_by!` method and continued testing for various security vulnerabilities, such as SQL injection, cross-site scripting (XSS), and command injection. These changes demonstrate a strong focus on improving the security of Ruby on Rails applications by enhancing the capabilities of the Brakeman security scanner and strengthening the test coverage for security-related issues. **Files Changed:** 1. `lib/brakeman/checks/check_unscoped_find.rb`: This file is part of the Brakeman security scanner, and the changes expand the check for unscoped ActiveRecord queries to include `find_by` and `find_by!` methods, in addition to `find` and `find_by_id`. The changes also handle optional `belongs_to` associations, further improving the scanner's ability to detect potential security vulnerabilities. 2. `test/apps/rails4/app/controllers/users_controller.rb`: The changes in this file update the `email_find_by` method to use the `find_by!` method instead of `find_by`, which can lead to more robust error handling and reduce the risk of SQL injection vulnerabilities. 3. `test/tests/rails4.rb`: This file contains the test suite for the Rails 4 application. The changes include the addition of a new test case for unscoped calls to the `Email#find_by!` method, as well as updates to the expected number of "generic" warnings. The existing test cases for various security vulnerabilities, such as SQL injection, XSS, and command injection, demonstrate a strong focus on application security.

Code Analysis

We ran 7 analyzers against 3 files and 0 analyzers had findings. 7 analyzers had no findings.

Riskiness

:green_circle: Risk threshold not exceeded.

View PR in the DryRun Dashboard.