Unvalidated redirect false negatives

[swallenfriedman]

Background

Brakeman version: 4.6.1 Rails version: 5.2.3 Ruby version: Using brakeman docker image

Issue

I noticed a few places where brakeman doesn't flag certain instances of unvalidated redirect vulnerabilities. Here's some example contrived controller code:

# Brakeman output for this file:
# == Warnings ==
# Confidence: High
# Category: Redirect
# Check: Redirect
# Message: Possible unprotected redirect
# Code: redirect_to(params)
# File: app/controllers/example_controller.rb
# Line: 22

# Confidence: Medium
# Category: Mass Assignment
# Check: MassAssignment
# Message: Parameters should be whitelisted for mass assignment
# Code: params.permit!
# File: app/controllers/example_controller.rb
# Line: 32

class AppplicationController < ActionController::Base
  # vulnerable to www.example.com/?host=http://www.evilexample.com
  def example_redirect_to_params
    redirect_to params
  end

  # vulnerable to www.example.com/?host=http://www.evilexample.com
  def example_redirect_to_request_params
    redirect_to request.params
  end

  # vulnerable to www.example.com/?host=http://www.evilexample.com
  def example_redirect_to_url
    redirect_to url_for(params.permit!)
  end

  # vulnerable to www.example.com/?host=http://www.evilexample.com
  def example_redirect_to_url_with_other
    redirect_to url_for(params.permit!.merge(other_param: value))
  end
end

Two of these 4 examples get correctly flagged, but all are vulnerable to open redirects (unless manually adding only_path: true as an additional parameter).

Thanks!

[presidentbeef]

Hi @swallenfriedman - thank you for reporting this! I suspect this is at least a little related to #1034 which I have not gone back to yet.

request.params is easy - looks like request.parameters and request.path_parameters are covered, just not request.params.

In any case - I will take a deeper look. Thanks!