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

Provide safer API for getting filenames of uploaded files #8008

Open
gmethvin opened this issue Nov 10, 2017 · 2 comments · May be fixed by #12183
Open

Provide safer API for getting filenames of uploaded files #8008

gmethvin opened this issue Nov 10, 2017 · 2 comments · May be fixed by #12183
Milestone

Comments

@gmethvin
Copy link
Member

The FilePart of MultipartFormData just uses the raw filename provided by the client. While this API is technically correct, it is very easy to mistakenly use the filename in unsafe places, such as in a path to write to a file on the system. This leaves users open to directory traversal attacks.

Instead of a filename method that returns the filename, we could perhaps have an unsafeFilename method and a sanitizedFilename method, and deprecate the existing filename method. The sanitized version could simply remove any other path components besides the actual file name, which is totally fine since the spec says the filename is only a suggested name.

@wsargent
Copy link
Member

There's a combination of normalize, resolve and startsWith in the Files NIO package which could be useful for this:

https://stackoverflow.com/questions/33083397/filtering-upwards-path-traversal-in-java-or-scala

@mkurz mkurz added this to the 2.9.0 milestone Feb 17, 2021
@mkurz mkurz modified the milestones: 2.9.0, Play.next Feb 11, 2022
@alexdboxall
Copy link
Contributor

Is this still an issue that needs fixing? If so, I'm happy to work on it

@alexdboxall alexdboxall linked a pull request Nov 7, 2023 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants