presidentbeef / brakeman

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

False negative with text_field_tag(name, raw(params[:something])) #739

Open oreoshake opened 9 years ago

oreoshake commented 9 years ago

Rails 4, automatic escaping turned on. Same result with html_safe

<%= text_field_tag :q, raw(params[:q]) %>

brakeman . -d

+------------------------------------------------------+
| Output                                               |
+------------------------------------------------------+
| [Escaped Output] text_field_tag(:q, raw(params[:q])) |
+------------------------------------------------------+

Output

<input type="text" name="q" id="q" value=""><script>alert(1)</script>" />
presidentbeef commented 9 years ago

Hm. Passing strings marked as safe to any method which automatically escapes them will cause them to not be escaped. I suppose Brakeman could look for any use of html_safe or raw on obvious user input like params?

oreoshake commented 9 years ago

If we can detect anything that's been raw'd or html_safe'd, we should always flag that in this case. The data must be escaped for it to be valid.

presidentbeef commented 9 years ago

@ytrezq please keep comments related to the issue or at least this project.