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

Fix for CVE-2021-22964 (4.4.1) mistreats valid URLs & erroneously returns '/' #246

Closed
2 tasks done
avoidwork opened this issue Oct 11, 2021 · 2 comments · Fixed by #247
Closed
2 tasks done

Fix for CVE-2021-22964 (4.4.1) mistreats valid URLs & erroneously returns '/' #246

avoidwork opened this issue Oct 11, 2021 · 2 comments · Fixed by #247

Comments

@avoidwork
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

3.x.x

Plugin version

4.4.1

Node.js version

any

Operating system

Linux

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

any version, any OS

Description

The fix for the redirects mishandles valid URLs by dropping the pathname & returning '/' early. Double slashes are valid as per the spec, with the exception that it's expected to include the authority component of the URI, however, NGINX and other popular servers treat the paths as equal to single slash and will return a file (200) if found at the expected folder; e.g. https://avoidwork.com///assets/img//avoidwork.svg is equal to https://avoidwork.com/assets/img/avoidwork.svg.

Steps to Reproduce

Visit any URL/URI served by a different & spec compliant server & modify it.

Expected Behavior

Consistent 1:1 path mapping for valid files on disk as other servers.

@mcollina
Copy link
Member

cc @Eomm

@avoidwork
Copy link
Author

Hi, I handled this issue via path.resolve(folder, decodeURIComponent(arg)); in one of my projects, arg being the pathname; then you can determine if valid or not.

@climba03003 climba03003 mentioned this issue Oct 11, 2021
4 tasks
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

Successfully merging a pull request may close this issue.

2 participants