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

Directory traversal with malformed HTTP request #330

Closed
hans362 opened this issue Apr 22, 2024 · 3 comments
Closed

Directory traversal with malformed HTTP request #330

hans362 opened this issue Apr 22, 2024 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@hans362
Copy link

hans362 commented Apr 22, 2024

Describe the bug
By sending a malformed HTTP request directly to webp_server_go, attackers can read images (but not files) outside IMG_PATH defined in configuration file.

To Reproduce
Suppose webp_server_go is serving at 127.0.0.1:23333 and IMG_PATH has been set to /opt/pics. Send the following HTTP request to the server. Note that the URI part does not start with /.

GET ../../test.png HTTP/1.1
Host: 127.0.0.1:23333

/test.png will be returned if exists, which is outside IMG_PATH.

Expected behavior
Images outside IMG_PATH should not be accessible.

Screenshots and logs
image

2024-04-22 22:06:06 time="2024-04-22 14:06:06" level=warning msg="can't read metadata: open metadata/local/f74fd045a66c476f.json: no such file or directory" func="[38:webp_server_go/helper.ReadMetadata]"
2024-04-22 22:06:06 time="2024-04-22 14:06:06" level=info msg="Resize /test.png itself to exhaust/local/f74fd045a66c476f" func="[87:webp_server_go/encoder.ResizeItself]"
2024-04-22 22:06:06  - [2024-04-22 14:06:05] GET ../../test.png 200 

Environment

  • OS: Any
  • Version: 0.11.2

Additional context
The patch for CVE-2021-46104 is not sufficient. path.Clean(reqUri) won't remove all ../ if reqUri begins with ../.

@BennyThink BennyThink added the bug Something isn't working label Apr 22, 2024
@n0vad3v
Copy link
Member

n0vad3v commented Apr 23, 2024

Thanks a lot for reporting this, will be solved in #331

We came up two ideas for mitigating this:

  1. Deny all request path that doesn't have prefix /
  2. Check if request path is starting with . or ./ as in current PR

Do you have any suggestion on this? @hans362

@hans362
Copy link
Author

hans362 commented Apr 23, 2024

I would suggest the first solution, which is to deny all request path that doesn't have prefix /, since this complies with RFC 7230 Section 5.3.1.

If the target URI's path component is empty, the client MUST send "/" as the path within the origin-form of request-target.

Meanwhile, this ensures that path.Clean() removes all ../ in the request path and will never traverse out of / so that it can be safely joined with IMG_PATH.

The second solution cannot mitigate this issue, since the following payload still works.

GET a/../../../test.png HTTP/1.1
Host: 127.0.0.1:23333

n0vad3v added a commit that referenced this issue Apr 23, 2024
* Directory traversal with malformed HTTP request

#330

* bump version

* also %2e

* Use prefix to check invalid Path

---------

Co-authored-by: n0vad3v <[email protected]>
@n0vad3v
Copy link
Member

n0vad3v commented Apr 23, 2024

Thank you for your explanation, I've updated PR and got it merged, the fix will be available in 0.11.3 (https://github.com/webp-sh/webp_server_go/releases/tag/0.11.3)

Thank you again for helping us to point out this vulnerability, which is very helpful to us! ❤️

@hans362 hans362 closed this as completed Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants