presidentbeef / brakeman

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

Revamp command injection in `pipeline*` calls #1868

Closed presidentbeef closed 2 months ago

presidentbeef commented 2 months ago

More specific argument checking for Open3.pipeline family of methods to execute shell commands.

Fixes #1862

dryrunsecurity[bot] commented 2 months ago

DryRun Security Summary

The provided code changes focus on improving the security of a Ruby on Rails application by addressing known vulnerabilities, enhancing the Brakeman security scanner, and identifying potential command injection issues in user-supplied input.

Expand for full summary
**Summary:** The provided code changes cover several important security-related updates across different components of a Ruby on Rails application. The key focus areas include: 1. **Brakeman Security Scanner Improvements**: The changes in the `check_execute.rb` file demonstrate enhancements to the Brakeman security scanner's ability to detect potential command injection vulnerabilities. This includes handling various system call methods, checking for specific patterns like "bash -c", and determining appropriate confidence levels for the reported issues. 2. **Rails 3 Security Vulnerabilities**: The changes in the `rails3.rb` test file address a wide range of known security vulnerabilities in Rails 3.0.3, including command injection, remote code execution, and session management issues. The comprehensive set of test cases suggests a thorough approach to identifying and mitigating these security risks. 3. **Potential Command Injection Vulnerabilities**: The changes in the `home_controller.rb` file introduce a new controller action that uses various `Open3` methods with user-supplied parameters. This can potentially lead to command injection, denial of service, information disclosure, and remote code execution vulnerabilities if the input is not properly validated and sanitized. Overall, these code changes demonstrate a strong focus on improving the security of the Ruby on Rails application by addressing known vulnerabilities and potential security issues. As an application security engineer, I would recommend thoroughly reviewing these changes, ensuring that all user input is properly validated and sanitized, and implementing appropriate security measures to mitigate the identified risks. **Files Changed:** 1. `lib/brakeman/checks/check_execute.rb`: This file contains changes to the Brakeman security scanner's command injection detection logic. The changes include handling various system call methods, checking for "bash -c" patterns, and determining appropriate confidence levels for reported issues. 2. `test/tests/rails3.rb`: This file includes updates to the test suite for addressing security vulnerabilities in a Rails 3 application. The changes cover command injection, remote code execution, session management, and mass assignment issues. 3. `test/apps/rails3/app/controllers/home_controller.rb`: This file introduces a new controller action called `test_more_uses_of_pipelines` that uses `Open3` methods with user-supplied parameters, which can potentially lead to command injection, denial of service, information disclosure, and remote code execution vulnerabilities.

Code Analysis

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

Riskiness

:green_circle: Risk threshold not exceeded.

View PR in the DryRun Dashboard.