Unprotected redirect not cured by only_path

Question

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?

score 10 · Accepted Answer · answered Sep 16 '14 at 22:18

10

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.

answered Sep 16 '14 at 22:18

Justin

1,561
10
12

This indeed satisfies Brakeman and is very sensible. Thank you! – tom_servo Sep 20 '14 at 00:04
1

I think it might be better not to use exceptions for control flow: ```if params[:referrer] =~ URI::regexp refer_url = URI.parse(params[:referrer]).path else refer_url = "some_default" end``` – haslo Nov 06 '14 at 09:52

Unprotected redirect not cured by only_path

1 Answers1