Skip to content

MultipartFile.getOriginalFilename() documentation should warn user not to use it as destination file name #26299

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

Closed
Marcono1234 opened this issue Dec 18, 2020 · 0 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task
Milestone

Comments

@Marcono1234
Copy link

Marcono1234 commented Dec 18, 2020

Affects: 5.3.2


The javadoc for method org.springframework.web.multipart.MultipartFile.getOriginalFilename() should contain a big warning that the file name is client-controlled and must not be used as part of the destination file name on the local disk.

An adversary can easily specify a malicious file name (cURL examples) and stripping off the directory name (as done by CommonsMultipartFile, see also #26207) is not enough (and might be disabled). For example the file name .. could cause issues as well when the code handling file uploads is supposed to replace an existing file, but in this case might actually end up deleting the parent directory. Similarly file names reserved under Windows could also cause issues.

It would therefore be best to advise the user not to use the file name (not even in combination with another string) as destination file name, but instead generate a random one and store the original file name somewhere else (if necessary).

Related OWASP links (ideally also link to them from the documentation):

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 18, 2020
@rstoyanchev rstoyanchev self-assigned this Jan 6, 2021
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 6, 2021
@rstoyanchev rstoyanchev added this to the 5.3.3 milestone Jan 6, 2021
rstoyanchev added a commit that referenced this issue Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

3 participants