presidentbeef / brakeman

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

False positive - unescaped model attribute with escape_javascript #1004

Open JasonBarnabe opened 7 years ago

JasonBarnabe commented 7 years ago
labels: [<%=data.keys.each.map{|d| '"' + escape_javascript(d.to_s) + '"'}.join(',').html_safe%>],

"Unescaped model attribute", but it's being escaped.

gutschi commented 5 years ago

@presidentbeef:

I noticed the same, .html_safe is not understood as proper escaping. Is that by design or should that be a feature request?

presidentbeef commented 5 years ago

@gutschi - can you be more specific? html_safe does not escape anything. It marks the string as "safe" which tells the templating language not to escape the string. In other words, html_safe is not safe.

In the context of the original post, values are escaped using escape_javascript. html_safe is ostensibly being using to avoid HTML escaping on top of the JavaScript escaping.

It's actually not clear from the context if that's safe or not, which may be why I have been extremely late in following up on it.