ruby-on-railsrubysecuritybrakeman

How to fix Brakeman redirect issue with multiple rest endpoints


I'm currently working on a solution for doing redirects in RoR because I got an error within the brakeman report saying that I have to fix redirects in a proper way. I understand what the message says and how to solve it within one controller action. But now I got the following. During the instantiation of the new method I set the HTTP_REFERER header which can be used in the create action.

This is giving me a Brakeman warning which can be found on the following link

Suppose I got the following controller with multiple endpoints:

 def new
      @my_model_set = MyModel.new
      @referer = request.env['HTTP_REFERER'] # We want to redirect to this referer after a create
 end
def create
  ...
  if @my_model_set.save
     flash_message :success, t('notification.item_created', type: @my_model_set.model_name.human)
     if params[:referer].present?
          redirect_to params[:referer]
     else
          redirect_to admin_my_model_set_path
     end
  else
  ...
  end
end

I already tried to fix this by using the redirect_back method from RoR but that's using the referer link of the create method which I don't want to use.

if @my_model_set.save
    flash_message :success, t('notification.item_created', type: @my_model_set.model_name.human)
    redirect_back(fallback_location: admin_my_model_set_path)
else
 ...
end


Solution

  • The main problem in your code is that params[:referer] can be set by your user (or an attacker forging a link for your user) to an arbitrary value by appending ?referer=https://malicious.site to the url. You will then redirect to that, which is an open redirect vulnerability.

    You could also argue that the referer header is technically user input, and you will be redirecting to it, but I would say in most cases and modern browsers that would probably be an acceptable risk, because an attacker does not really have a way to exploit it (but it might depend on the exact circumstances).

    One solution that immediately comes to mind for similar cases would be the session - but on the one hand this is a rest api if I understand correctly, so there is no session, and on the other hand, it would still not be secure against an attacker linking to your #new endpoint from a malicious domain.

    I think you should validate the domain before you redirect to it. If there is a common pattern (like for example if all of these are subdomains of yourdomain.com), validate for that. Or you could have your users register their domains first before you redirect to it (see how OAuth2 works for example, you have to register your app domain first before the user can get redirected there with a token).

    If your user might just come from anywhere to #new and you want to send them back wherever they came from - that I think is not a good requirement, you should probably not do that, or you should carefully assess the risk and consciously accept it if you want to for some reason. In most cases there is a more secure solution.