presidentbeef / brakeman

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

Dynamic Render Path false positive #1414

Open grantbdev opened 4 years ago

grantbdev commented 4 years ago

Background

Brakeman version: 4.7.0 Rails version: 5.2.3 Ruby version: 2.5.3

False Positive

Full warning from Brakeman:

Confidence: Weak
Category: Dynamic Render Path
Check: Render
Message: Render path contains parameter value
Code: render(action => user_path(User.find(params[:id])), { :method => "patch" })

Relevant code:

<%= render "form",
  action: user_path(User.find(params[:id])),
  method: "patch"
%>

Using action as a keyword for one of the locals in the partial to be rendered appears to be making Brakeman think that the value of action is the thing being rendered. So when the value of the action local contains a parameter value I think that triggers a false positive without realizing there the first argument is a static string for the partial name. If I rename the local from action to something else like form_action, Brakeman doesn't give a warning.

presidentbeef commented 4 years ago

Hm, yes it looks like right now if there is a hash passed in it will override any inferred information about the render call.