Search code examples
ruby-on-rails-4brakeman

Unprotected redirect not cured by only_path


I have a Rails 4 application, and when I run Brakeman, it (rightly) identifies an unprotected redirect in my create action. However, adding only_path: true (as in the Brakeman Railscast) does not cure the warning:

  def create
    refer_url = params[:referrer]
    @portfolio = current_user.portfolios.build(portfolio_params)
    if @portfolio.save
      redirect_to refer_url, notice: "Portfolio was successfully created.", only_path: true
    else
      render :new
    end
  end

Results in:

+SECURITY WARNINGS+

+------------+-----------------------+---------+--------------+----------------------------------------------------------------------------------------------------------------------->>
| Confidence | Class                 | Method  | Warning Type | Message                                                                                                               >>
+------------+-----------------------+---------+--------------+----------------------------------------------------------------------------------------------------------------------->>
| High       | PortfoliosController  | create  | Redirect     | Possible unprotected redirect near line 14: redirect_to(+params[:referrer]+, :notice => "Portfolio was successfully cr>>
+------------+-----------------------+---------+--------------+----------------------------------------------------------------------------------------------------------------------->>

Why might this be? What risk is Brakeman still identifying?


Solution

  • The RailsCast is incorrect, unfortunately. :only_path => true must be part of the first argument.

    Is params[:referrer] supposed to be a path in your application?

    If so, this would be my recommendation:

    begin
      refer_url = URI.parse(params[:referrer]).path
    rescue URI::InvalidURIError
      refer_url = "some_default"
    end
    

    Or you could check that params[:referrer] is always a path, validate it some other way, or just don't allow arbitrary redirects even within your application. Sadly, Rails does not give easy options for safe redirects.