Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

URL path traversal attacks can cause downcase ArgumentError #64

Open
volleio opened this issue Dec 14, 2023 · 3 comments
Open

URL path traversal attacks can cause downcase ArgumentError #64

volleio opened this issue Dec 14, 2023 · 3 comments

Comments

@volleio
Copy link

volleio commented Dec 14, 2023

On v2.0.0, navigating to a URl that contains a directory traversal attack in its path (e.g. localhost:5000/%c0%ae%c0%ae) will cause the following error to be raised:

ArgumentError: input string invalid
/usr/local/bundle/gems/route_downcaser-2.0.0/lib/route_downcaser/downcase_route_middleware.rb:64:in `downcase': input string invalid (ArgumentError)

Calling URI.decode_www_form_component("%c0") returns "\xC0", which is considered an invalid string for downcasing.

@koppen
Copy link
Member

koppen commented Dec 15, 2023

Hey, thanks for reporting this. I wonder what the actual behavior should be, what'd you expect?

@volleio
Copy link
Author

volleio commented Dec 15, 2023

Good point, and I would understand if the solution should be for the application/firewall to sanitize input beforehand.

I would expect those characters to be ignored by the downcase, which could be done with downcase(:ascii). Will have to check if that's compatible with ruby 3+

@koppen
Copy link
Member

koppen commented Dec 16, 2023

downcase(:ascii) doesn't work for other valid cases, though. I would expect "ÆBLEGRØD" to downcase to "æblegrød", but:

"ÆBLEGRØD".downcase(:ascii) #=> "ÆblegrØd"

Would it make sense to catch the error and return the original string? In other words, if the URL isn't valid for downcasing we effectively ignore it? For attack-like URLs like the example that makes sense, I reckon, and it should be backwards compatible, given that it's a case we can't handle currently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants