Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revamp command injection in pipeline* calls #1868

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

presidentbeef
Copy link
Owner

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

Fixes #1862

Copy link

dryrunsecurity bot commented Sep 1, 2024

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

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

@presidentbeef presidentbeef merged commit e4f49f6 into main Sep 3, 2024
18 checks passed
@presidentbeef presidentbeef deleted the revamp_pipeline_check branch September 3, 2024 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False positive when passing a command array to Open3.pipeline
1 participant