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 when early-returning in unless block #1073

Closed
saifulwebid opened this Issue Aug 7, 2017 · 1 comment

Comments

Projects
None yet
2 participants
@saifulwebid

saifulwebid commented Aug 7, 2017

This is an extension from #1057, but I early-return like this:

bank_name = params_filename.first
unless Statistics::AdminWithdrawal::BANK_LIST.include?(bank_name)
  flash[:alert] = 'Invalid filename'
  redirect_to :back
  return
end

Statistics::AdminWithdrawal.send("export_#{bank_name}_inc!")

Brakeman reports this as a dangerous send, even if the send() method will only be called if it passes the unless block (because there's a return there as a final statement).

@presidentbeef

This comment has been minimized.

Show comment
Hide comment
@presidentbeef

presidentbeef Aug 9, 2017

Owner

Yes...it is possible to support this case (assuming Brakeman can find Statistics::AdminWithdrawal::BANK_LIST).

However, I do want to point out for future people looking at these issues that Brakeman is not going to support all possible instances of early returns.

Owner

presidentbeef commented Aug 9, 2017

Yes...it is possible to support this case (assuming Brakeman can find Statistics::AdminWithdrawal::BANK_LIST).

However, I do want to point out for future people looking at these issues that Brakeman is not going to support all possible instances of early returns.

presidentbeef added a commit that referenced this issue Aug 13, 2017

Repository owner locked and limited conversation to collaborators Sep 25, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.