Ensure exact match when rejecting global excludes with EXCLUDED_PATHS

[gazayas]

Closes #1830.

We can scaffold a simple, namespaced model to test the new changes.

# Rails Version: 7.1.4.2
# Ruby Version: 3.3.1

> rails new brakeman_test
> cd brakeman_test
> bundle add brakeman --git "https://github.com/gazayas/brakeman.git" --branch "fixes/update-excluded-paths-strings"
> rails g scaffold Catalog::Order title:string
> rails db:migrate

<%# Add command injection to `app/views/catalog/orders/_order.html.erb` %>
<%= `#{x}` %>

Due to the problem in #1830 The current version of brakeman (6.2.2) won't report any warnings:

== Overview ==

Controllers: 1
Models: 1
Templates: 2
Errors: 0
Security Warnings: 0

== Warning Types ==


No warnings found

However, with the changes in this PR we get the following output.

== Overview ==

Controllers: 2
Models: 2
Templates: 8
Errors: 0
Security Warnings: 1

== Warning Types ==

Command Injection: 1

== Warnings ==

Confidence: Medium
Category: Command Injection
Check: Execute
Message: Possible command injection
Code: `#{x}`
File: app/views/catalog/orders/_order.html.erb
Line: 9

Experimenting with Regular Expressions

Since the value of relative_path often times does not start with a leading /, I chose to add one at the beginning when it doesn't exist. That way we can be sure we're getting an exact match for the directories we need to exclude.

I primarily was thinking about scanning for exact matches with a Regular Expression instead of adding the leading /:

# Since we would get only directories with the following,
# it seemed like a good idea at first...
File.dirname(relative_path).split("/")
#=> ["app", "views", "catalog", "orders"]

However, since we have nested directories like lib/tasks/ and lib/templates/ in EXCLUDED_PATHS, I experimented with some regular expressions to extract those two by themselves. I ultimately refrained from this because I felt it made the code more convoluted/hard to read, so I left things simple and just went with the leading / in EXCLUDED_PATHS. I hope this helps!

[dryrunsecurity]

DryRun Security Summary

The pull request updates the Brakeman application security scanner's AppTree class to improve the reliability and consistency of the tool by updating the EXCLUDED_PATHS constant and adding a check to ensure that the relative path starts with a slash before checking if it includes any of the excluded paths.

Expand for full summary

Summary:

The code changes in this pull request are related to the Brakeman application security scanner's AppTree class, which is responsible for managing the file paths and directories within a Rails application. The key changes include updating the EXCLUDED_PATHS constant to include leading slashes for the directories that should be excluded from the file path search, and adding a check to ensure that the relative path starts with a slash before checking if it includes any of the excluded paths.

These changes are not directly related to security vulnerabilities, but they do improve the reliability and consistency of the Brakeman tool, which is an important part of an application security program. The AppTree class is responsible for identifying the relevant files and directories within a Rails application, which is a crucial step in the Brakeman scanning process. Ensuring that the file path handling is robust and accurate helps to improve the overall effectiveness of the Brakeman tool in identifying potential security issues.

Files Changed:

• lib/brakeman/app_tree.rb: This file contains the AppTree class, which is responsible for managing the file paths and directories within a Rails application. The changes in this pull request include:
  1. Updating the EXCLUDED_PATHS constant to include leading slashes for the directories that should be excluded from the file path search.
  2. Adding a check to ensure that the relative path starts with a slash before checking if it includes any of the excluded paths.

These changes improve the reliability and consistency of the Brakeman tool, which is an important part of an application security program.

Code Analysis

We ran 9 analyzers against 1 file and 0 analyzers had findings. 9 analyzers had no findings.

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.