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 #700 - Arbitrary File Read Vulnerability #701

Merged
merged 2 commits into from
Nov 6, 2016

Conversation

perwendel
Copy link
Owner

Fix for #700
This is my take on the directory traversal problem.
Please review and try out.

@perwendel
Copy link
Owner Author

perwendel commented Nov 5, 2016

I've just verified it for Windows, didn't have time to test for linux etc.
When travis CI built it didn't detect directory traversal for the test case for external resources. There's probably an issue with how path handling differs in unix-like OSs and windows.

@jakaarl
Copy link
Contributor

jakaarl commented Nov 6, 2016

DISCLAIMER: Didn't really have time to look closely, so I could be talking out of my arse so to speak, but: wouldn't using File.getCanonicalPath() be safer than string manipulation? I'd certainly trust JDK library methods more than my own code, especially when it comes to security. Also, there are all kinds of clever ways (encodings, escapes and what not) of defeating checking strings for "..", "/", "" etc.

@perwendel perwendel merged commit efcb46c into master Nov 6, 2016
@perwendel
Copy link
Owner Author

@jakaarl Sorry, merged before I saw your comment. The guy making the post emailed and said it was a good solution. But I'll follow your advise and see if I can make it better. New PR will be created if you're right!

@tipsy tipsy deleted the fix-for-directory-traversal-issue-700 branch August 15, 2017 18:02
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 this pull request may close these issues.

None yet

2 participants