Skip to content

Raise BadRequest error when escaping invalid URLs

Joseph Wambua requested to merge jj-382599-AMS-21035-jira-xss-header-injection-b into master

What does this MR do and why?

In Escape Snowplow asset url to prevent XSS Header... (!108211 - merged), we added the ability to escape urls, especially when used to import assets in order to prevent malicious javascript execution by XSS header injection.

Based on this comment it would be nice to return 400 (Bad Request) errors instead of generic 500 errors.

Related to: https://gitlab.com/gitlab-org/gitlab/-/issues/382599+

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

How to set up and validate locally

To demonstrate this problem and fix locally, I followed these steps (on Google Chrome):

(These steps assume that the local test is accessible at http://gdk.test:3000)

Before

  1. Install the ModHeader extension
  2. Enable Snowplow tracking following the docs
  3. Add an X-Forwarded-Host header to Request headers with an invalid URL as the value (e.g.):
    • injected_here"+eval(1)+"
  4. With the gdk running on the master branch, navigate to http://gdk.test:3000/explore
  5. You will encounter an Invalid URL error.

note: It might be needed to allow the host, update the development.rb with the URL matching the one in step 3:

  • config.hosts << 'injected_here"+eval(1)+"'

After

  1. Switch the GDK to the jj-382599-AMS-21035-jira-xss-header-injection-b branch with the fix
  2. Navigate to http://gdk.test:3000/explore
  3. The resulting error should be a 400: Bad Request error

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Joseph Wambua

Merge request reports