https://github.com/presidentbeef/brakeman/issues/1841

[kwerle]

brakeman still references haml 4 - which is a bit long in the tooth (Haml::Filter::Coffee class vs. module) #1841

[dryrunsecurity]

Hi there 👋, @DryRunSecurity here, below is a summary of our analysis and findings.

DryRun Security	Status	Findings
Configured Codepaths Analyzer	✅	0 findings
Sensitive Files Analyzer	✅	0 findings
Authn/Authz Analyzer	✅	0 findings
AppSec Analyzer	✅	0 findings
Secrets Analyzer	✅	0 findings

Note

🟢 Risk threshold not exceeded.

Powered by DryRun Security

[presidentbeef]

Hmmm... I kind of think if Haml has these filters installed by default now, that the fake filters aren't needed? 🤔 And I think they were added in 6.0 when Hamlit became Haml?

If I recall correctly, the fake filters were just to stub out the embedding of other languages and avoid raising errors. It wasn't to override the filters.

[kwerle]

Hmmm... I kind of think if Haml has these filters installed by default now, that the fake filters aren't needed? 🤔 And I think they were added in 6.0 when Hamlit became Haml?

If I recall correctly, the fake filters were just to stub out the embedding of other languages and avoid raising errors. It wasn't to override the filters.

I'm a little lost. What is the correct solution to #1841?

[kwerle]

Specifically, we would like to use brakeman-min (6.1.2 or so) with haml (6.3.0) and we can't because brakeman refers to these constants as modules and haml declares them as classes. I can't think of any solution to this other than the one I've proposed (short of forking brakeman - which seems ridiculous).

Our version of brakeman-min is a little behind, but this is the most recent version of haml.

[presidentbeef]

I might have to look again, but I believe I was thinking instead of defining the classes if defined?(Haml::Filters::Coffee) && Haml::Filters::Coffee.class == Class to only define the modules if the classes aren't available.

In other words, for newer Haml Brakeman doesn't need to do anything, just skip defining the modules.

[kwerle]

In other words, for newer Haml Brakeman doesn't need to do anything, just skip defining the modules.

I see! I can take a poke at that.

[dryrunsecurity]

DryRun Security Summary

The provided code change is a maintenance update to the Brakeman project, a security scanner for Ruby on Rails applications, which involves handling the parsing of HAML files that contain embedded code, including the definition of "fake" filters for CoffeeScript, Markdown, and Sass code embedded within HAML templates.

Expand for full summary

Summary:

The provided code change is related to the Brakeman project, a security scanner for Ruby on Rails applications. The change is made to the lib/brakeman/parsers/haml_embedded.rb file, which is responsible for handling the parsing of HAML (HTML Abstraction Markup Language) files that contain embedded code. The key change is the handling of the Haml::Filters::Coffee module, which is now checked to see if it is defined as a class. If it is defined as a module, the code proceeds to define the Haml::Filters::Coffee, Haml::Filters::Markdown, and Haml::Filters::Sass modules, which are "fake" filters for HAML. These filters are used to handle the compilation of CoffeeScript, Markdown, and Sass code embedded within HAML templates.

From an application security perspective, this change does not appear to introduce any significant security concerns. However, it's worth noting that the use of "fake" filters for handling embedded code could potentially introduce some complexity and maintainability issues if not properly managed. It's important to ensure that these filters are kept up-to-date and properly tested to avoid any potential security vulnerabilities that could arise from the handling of embedded code. Overall, this code change seems to be a maintenance update to the Brakeman project, and it does not appear to introduce any obvious security risks.

Files Changed:

• lib/brakeman/parsers/haml_embedded.rb: This file is responsible for handling the parsing of HAML files that contain embedded code. The key change is the handling of the Haml::Filters::Coffee module, which is now checked to see if it is defined as a class. If it is defined as a module, the code proceeds to define the Haml::Filters::Coffee, Haml::Filters::Markdown, and Haml::Filters::Sass modules, which are "fake" filters for HAML. These filters are used to handle the compilation of CoffeeScript, Markdown, and Sass code embedded within HAML templates.

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.

[kwerle]

OK, the good news is that I think that will all work. The .. bad? new is that brakeman is locked to haml 5.1 so I'm having to do some other gymnastics to work around this issue anyway. Mostly it means no longer including brakeman in our gemfile (probably the right thing to do) and just run it from a container.

So... victory? I guess merge this if you think it's the right thing.

And thank you!

[presidentbeef]

Yes, looks like there are some breaking changes with Haml 6.x that need to be addressed.