Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positive dangerous send case when #1730

Closed
ngouy opened this issue Sep 13, 2022 · 3 comments · Fixed by #1734
Closed

False positive dangerous send case when #1730

ngouy opened this issue Sep 13, 2022 · 3 comments · Fixed by #1734

Comments

@ngouy
Copy link

ngouy commented Sep 13, 2022

Background

Rails Version: 7.0.3.1
Brakeman Version: 5.3.1
ruby version: ruby 2.7.5p203 (2021-11-24 revision f69aeb8314) [x86_64-darwin20]

I'm using a case;when with predefined values to make sure the value is matching a specific finite list of values.
Then using a send (tried with public_send and send) but brakeman is not happy with that.

I'm not sure if:

  • it's a bug (I think so)
  • it's fixable
  • or maybe meant to work like that

Issue

False positive dangerous send:

My code:

AVAILABLE_TIME_STEPS = ["week", "day", "year"]

value = params.observation_window.fetch("value")
step = params.observation_window.fetch("step")

case step
when "last_n"
  offset = [scope.count - value, 0].max
  scope.offset(offset).limit(value)
when *AVAILABLE_TIME_STEPS
  time_ago = value.public_send(step).ago(range_end)
  range = (time_ago..range_end)
  scope.where(observed_date: range)
else
  raise "unknown step"
end

Other Error

Run Brakeman with --debug to see the full stack trace.

Stack trace:

trace:

== Warnings ==

Confidence: High
Category: Dangerous Send
Check: Send
Message: User controlled method execution
Code: params.observation_window.fetch("value").public_send(params.observation_window.fetch("step"))
File: app/lib/workflows/matchers/observation.rb
Line: 103
@presidentbeef
Copy link
Owner

I suspect Brakeman just isn't handling the splat case right now. If you change the code to

when ["week", "day", "year"]

It will not produce a warning. If that's the case, then this should be fixable in Brakeman.

@ngouy
Copy link
Author

ngouy commented Sep 27, 2022

I've added it to the ignore file tbh
If you think it's not something brakeman should handle, feel free to close

Thanks for the reply

@ngouy
Copy link
Author

ngouy commented Oct 11, 2022

🔥

Repository owner locked and limited conversation to collaborators May 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants