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 for "Possible unprotected redirect" #1117

Closed
obfuscoder opened this Issue Nov 10, 2017 · 1 comment

Comments

Projects
None yet
3 participants
@obfuscoder

obfuscoder commented Nov 10, 2017

Our rails app initializes sessions via backend and stores the session token in the database. This session token is then used by the client to initiate the frontend by calling

http://theapp/?session=thetoken

We are using something like this to lookup whether we have a session with this token:

session = Session.find_by_token params[:session]
if session
  # proceed with extracting user context from session and more and redirect to the last path the user was shown to
  login(session.user)
  redirect_to session.user.current_path
else
  redirect_to expired_or_invalid_session_path
end

brakeman reports this:

Confidence: High
Category: Redirect
Check: Redirect
Message: Possible unprotected redirect
Code: redirect_to(Session.find_by_token(params[:session]).user.current_path)
File: ...rb
Line: ...

This seems to be a false positive. The parameter is checked by querying the database for finding an object with that session token. The tokens are randomly generated strings with high entropy. I would not expect brakeman to report this as the indirect lookup via DB is performed.

@presidentbeef presidentbeef added this to the 4.0.2 milestone Nov 17, 2017

@codebycliff

This comment has been minimized.

codebycliff commented Feb 8, 2018

I believe I am getting a false positive too. Here is the warning I'm getting:

Confidence: High
Category: Redirect
Check: Redirect
Message: Possible unprotected redirect
Code: redirect_to(TeamUser.find(params[:id]).team.challenge, :notice => ("You have been added to team #{TeamUser.find(params[:id]).team.name}"))
File: app/controllers/team_users_controller.rb
Line: 9

If this isn't a false positive and there is something I am doing wrong, please let me know.

presidentbeef added a commit that referenced this issue Feb 21, 2018

Repository owner locked and limited conversation to collaborators May 9, 2018

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