Skip to content

Rack path traversal: exclude filter_projects query parameters

David Fernandez requested to merge 463002-exclude-filter-projects into master

🌳 Context

Similar to Rack path traversal: exclude search query para... (!153078 - merged).

We are slowing rolling out the Rack middleware for path traversal checks. See https://gitlab.com/groups/gitlab-org/-/epics/13437+.

The middleware is only logging attempts. It is behind a feature flag.

During the roll out, we noticed that we could have an issue with the search feature in the project forks page. Basically, search terms are usually free text and it's generally agreed that users can put whatever text they want.

As such, we should avoid running the middleware on the query parameters that are used for the search. In Rack path traversal: exclude search query para... (!153078 - merged), we excluded a set of query parameters that are known to be used in searches. We will need to add filter_projects to that set.

That's what this MR does. The related issue is https://gitlab.com/gitlab-org/gitlab/-/issues/463002+.

What does this MR do and why?

  • Exclude the filter_projects query parameter in the check path traversal middleware.

Note that specs were not updated because the excluded query parameter names are in the constant and we have a spec that will loop on that constant to assert the behavior.

The entire middleware is behind a feature flag. At the time of this writing, we're still rolling it out.

🏁 MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

🖥 Screenshots or screen recordings

🤷 (No UI changes)

How to set up and validate locally

  1. Enable the middleware: Feature.enable(:check_path_traversal_middleware)
  2. Have a project ready with a git repository initialized.

1️⃣ On master

Browse <project full path>/-/forks?filter_projects=foo%2F..%2Fbar.

Look at log/application_json.log:

{"severity":"WARN","time":"2024-05-22T10:49:14.742Z","correlation_id":"01HYFYPRAGEN77S767B6RXKC78","meta.caller_id":"Projects::ForksController#index","meta.remote_ip":"172.16.123.1","meta.feature_category":"source_code_management","meta.user":"root","meta.user_id":1,"meta.project":"root/dependency-proxy-performance-investigation2","meta.root_namespace":"root","meta.client_id":"user/1","method":"GET","fullpath":"/root/dependency-proxy-performance-investigation2/-/forks?filter_projects=foo%2F..%2Fbar","message":"Potential path traversal attempt detected","status":200,"class_name":"Gitlab::Middleware::PathTraversalCheck"}

An attempt has been caught.

2️⃣ With this MR

Browse <project full path>/-/forks?filter_projects=foo%2F..%2Fbar again

Look at log/application_json.log:

(empty)

The middleware properly excluded the filter_projects parameter 🎉

Edited by David Fernandez

Merge request reports