Skip to content

Conversation

khaf
Copy link

@khaf khaf commented Jun 6, 2017

This PR fixes an issue where encoded URLs could escape the static directory and traverse the filesystem.

@khaf khaf changed the title Security Fix: Filesystem traversal big Security Fix: Filesystem traversal bug Jun 6, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 81.846% when pulling 5577b3a on khaf:fix_static_traversal into c3887eb on labstack:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 81.846% when pulling 5577b3a on khaf:fix_static_traversal into c3887eb on labstack:master.

@vishr
Copy link
Member

vishr commented Jun 6, 2017

@khaf Can you provide an example or better may be a test case to understand and document it better?

@khaf
Copy link
Author

khaf commented Jun 6, 2017

Sure, these are the type of URLs that may hit the server:

[*] Testing Path: http://127.0.0.1:8081/..%2f..%2f..%2fetc%2fissue <- VULNERABLE!
[*] Testing Path: http://127.0.0.1:8081/..%2f..%2f..%2f..%2fetc%2fpasswd <- VULNERABLE!
[*] Testing Path: http://127.0.0.1:8081/..%2f..%2f..%2f..%2fetc%2fissue <- VULNERABLE!
[*] Testing Path: http://127.0.0.1:8081/..%2f..%2f..%2f..%2f..%2fetc%2fpasswd <- VULNERABLE!

These are out of the following tool: https://github.com/wireghoul/dotdotpwn

This happens as a result of not sanitizing the encoded input (in this case URL path) before passing it to the path.Clean method.

@vishr vishr added the bug label Jun 6, 2017
@vishr
Copy link
Member

vishr commented Jun 6, 2017

@khaf Nice! Just need to write a test case and I will get it merged asap.

@vishr vishr closed this in bd96cc3 Jun 6, 2017
@vishr
Copy link
Member

vishr commented Jun 6, 2017

@khaf This is fixed differently, please verify.

vishr added a commit that referenced this pull request Jun 7, 2017
Signed-off-by: Vishal Rana <vr@labstack.com>
@khaf
Copy link
Author

khaf commented Jun 7, 2017

Looks like it is fixed now. Thanks.

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

Successfully merging this pull request may close these issues.

3 participants