Rack path traversal: exclude search query parameters

🔭 Context

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 search pages. 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 those pages. That is also why the global search urls have been excluded from the middleware. These exclusion works with url prefixes. For example, if we exclude /foo, then everything below /foo (like /foo/bar) would be excluded.

In https://gitlab.com/gitlab-org/gitlab/-/issues/460268+, we are trying to come up with a different approach that could help ups avoid the issues with search terms but still cover sub urls.

The idea is: instead of excluding url prefixes, look at the query parameters. Those that are known to be used in search pages, ignore them.

🤔 What does this MR do and why?

• Update the middleware path traversal check:
  • Do not exclude the global search urls.
  • Parse the query params, exclude the ones know to be used in search pages and update the (cloned) env in the rack request.
• Update the related specs.

Again, the entire middleware is behind a feature flag that is currently being rolled out. At the time of this writing we are at 10% of the requests.

🏁 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

This is a middleware so 🤷

⚗ How to set up and validate locally

1. Enable the feature flag : Feature.enable(:check_path_traversal_middleware)
2. Browse the following urls:
   • global search: http://gdk.test:8000/search?search=foo%2F..%2Fbar
   • projects search: http://gdk.test:8000/?name=foo%2F..%2Fbar&sort=latest_activity_desc
   • groups search: http://gdk.test:8000/dashboard/groups?filter=foo%2F..%2Fbar
   • issues search: http://gdk.test:8000/dashboard/issues?sort=created_date&state=opened&assignee_username[]=root&search=foo/../bar
   • MR search: http://gdk.test:8000/dashboard/merge_requests?scope=all&state=opened&assignee_username=root&search=foo%2F..%2Fbar
   • Packages search: http://gdk.test:8000/flightjs/Flight/-/packages?orderBy=created_at&sort=desc&search[]=foo%2F..%2Fbar
   • Autocomplete search: Click on Search or go to... and type a word.
3. Check log/application_json.log

1️⃣ On Master

• global search: http://gdk.test:8000/search?search=foo%2F..%2Fbar -> no attempt logged ✅
• global search sub url: http://gdk.test:8000/search/foo%2F..%2Fbar?search=foo%2F..%2Fbar -> no attempt logged ❌
• projects search: http://gdk.test:8000/?name=foo%2F..%2Fbar&sort=latest_activity_desc -> attempt logged ❌
• groups search: http://gdk.test:8000/dashboard/groups?filter=foo%2F..%2Fbar -> attempt logged ❌
• issues search: http://gdk.test:8000/dashboard/issues?sort=created_date&state=opened&assignee_username[]=root&search=foo/../bar -> attempt logged ❌
• MR search: http://gdk.test:8000/dashboard/merge_requests?scope=all&state=opened&assignee_username=root&search=foo%2F..%2Fbar -> attempt logged ❌
• Packages search: http://gdk.test:8000/flightjs/Flight/-/packages?orderBy=created_at&sort=desc&search[]=foo%2F..%2Fbar -> attempt logged ❌
• Autocomplete search: Click on Search or go to... and type foo/../bar. -> no attempt logged ✅

2️⃣ With this MR

• global search: http://gdk.test:8000/search?search=foo%2F..%2Fbar -> no attempt logged ✅
• global search sub url: http://gdk.test:8000/search/foo%2F..%2Fbar?search=foo%2F..%2Fbar -> attempt logged ✅
• projects search: http://gdk.test:8000/?name=foo%2F..%2Fbar&sort=latest_activity_desc -> no attempt logged ✅
• groups search: http://gdk.test:8000/dashboard/groups?filter=foo%2F..%2Fbar -> no attempt logged ✅ -> no attempt logged ✅
• issues search: http://gdk.test:8000/dashboard/issues?sort=created_date&state=opened&assignee_username[]=root&search=foo/../bar -> no attempt logged ✅
• MR search: http://gdk.test:8000/dashboard/merge_requests?scope=all&state=opened&assignee_username=root&search=foo%2F..%2Fbar -> no attempt logged ✅
• Packages search: http://gdk.test:8000/flightjs/Flight/-/packages?orderBy=created_at&sort=desc&search[]=foo%2F..%2Fbar -> no attempt logged ✅

🔮 Conclusions

We can see that the middleware behavior has been changed in the way we want:

• search query parameters are ignored -> no attempt logged the issue is located there.
• global search is protected on query parameters only -> sub urls are now protected -> sub urls with a path traversal attempt will be properly logged.
• Autocomplete search: Click on Search or go to... and type a word. -> no attempt logged ✅