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

Different URL parsing between fastify and URL whatwg standard #5204

Closed
2 tasks done
stefanbeigel opened this issue Dec 11, 2023 · 6 comments
Closed
2 tasks done

Different URL parsing between fastify and URL whatwg standard #5204

stefanbeigel opened this issue Dec 11, 2023 · 6 comments

Comments

@stefanbeigel
Copy link

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.x.x

Plugin version

x.x.x

Node.js version

20.x

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

22.04

Description

Hi,
when using a http client that parses the URL with the whatwg URL class ../ and ./ is evaluated as described in the whatwg standard:
https://url.spec.whatwg.org/#url-representation

A single-dot URL path segment is a URL path segment that is "." or an ASCII case-insensitive match for "%2e".
A double-dot URL path segment is a URL path segment that is ".." or an ASCII case-insensitive match for ".%2e", "%2e.", or "%2e%2e".

If buffer is a double-dot URL path segment, then Shorten url’s path.

What this has to do with fastify?
If a requests hits fastify ../ and ./ are not evaluated, all the routing is done with the original path.
If you then forward the call to another service using request.raw.url with a http client that uses the whatwg url class, a path traversal attack is possible.
So a completly other path can be called on the target service.

Steps to Reproduce

I have prepared a sample application.
https://github.com/stefanbeigel/whatwg-fastify-path-traversal/blob/main/index.mjs
Call the app with curl --path-as-is localhost:3000/abc/../foobar

Expected Behavior

Maybe fastify could parse the incoming url with a whatwg compliant url parser and resolve ../ and ./ before the handler matching is done and also put the parsed url into request.raw.url

@stefanbeigel stefanbeigel changed the title Possible path traveral vulnerability when using fastify with http clients that use WHATWG url Possible path traversal vulnerability when using fastify with http clients that use WHATWG url Dec 11, 2023
@mcollina
Copy link
Member

Security reports should follow the security process at https://github.com/fastify/fastify/blob/main/SECURITY.md. Reporting them in public poses a significant threat to the community and likely your employer as well (if you are using Fastify in production). Causing data leaks and security incidents for your employer could also be a legitimate firing offense in a few legislation - or at least a bad mark in you employment contract.

Luckily for you, this is not one. There are almost infinite ways that a .. could be exploited somewhere when you are not validating your input data. If you are forwarding the request somewhere else, you likely should be doing that.

@stefanbeigel
Copy link
Author

stefanbeigel commented Dec 11, 2023

Hi Matteo and thanks for your response,
I agree that this is a validation problem, but why should I validate the path again when it was already matched by fastify route.
I think the current situation is not good at all, because the developer needs to know about the different parsing behaviors.
What would you recommend now? Should I open an issue for WHATWG?
BR

@mcollina
Copy link
Member

mcollina commented Dec 12, 2023

Take a look at https://owasp.org/www-community/attacks/Path_Traversal. Essentially it shows quite an extensive list of possible path traversal attacks. What you do with inputs is up to you.

@mcollina mcollina closed this as not planned Won't fix, can't repro, duplicate, stale Dec 12, 2023
@Rand0mF
Copy link

Rand0mF commented Dec 13, 2023

@mcollina I think it's not really about the path traversal, but rather that fastify uses a different url parsing than nodejs itself for route matching. If you have an inbound call with http://localhost:3000/abc/../foobar it should not match the /abc route defined in fastify as can be seen if the url is put to new URL()
image

@stefanbeigel stefanbeigel changed the title Possible path traversal vulnerability when using fastify with http clients that use WHATWG url Different URL parsing between fastify and URL whatwg standard Dec 28, 2023
@stefanbeigel
Copy link
Author

Hi @mcollina
I agree it's not vulnerability but it's still an issue of fastify as a different URL parsing is used and not the what wg URL standard.
As @Rand0mF stated fastify should not match the /abc route, but the /foobar route.
I changed the title to better reflect the problem, could you please reopen the issue?
BR

@mcollina
Copy link
Member

I don't think we plan to change the algorithm. More importantly, there are valid use cases for '..' not being interpreted like that.

Fastify never aimed to be compatible with the WHATWG Url parsing standard.

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

3 participants