I sent this report in an Email to firstname.lastname@example.org on 2017-10-16. I did not receive any kind of response yet (2017-11-18 as of writing). I am now posting it publicly, to open the issue up for discussion, and hopefully initiate a fix.
This report is not about a vulnerability in commons-io per se, but an unexpected behavior that has a high chance of introducing a path traversal vulnerability when using FileNameUtils.normalize to sanitize user input. The traversal is limited to a single out-of-bounds-stepping "/../" segment.
Consider a web-application that uses FileNameUtils.normalize to sanitize a user-supplied file name string, and then appends the sanitized value to a configured upload directory to store the uploaded content in:
- org.apache.commons.io.FilenameUtils#getPrefixLength : everything between a leading "//" and the next "/" is treated as a UNC server name, and ignored in all further validation logic of org.apache.commons.io.FilenameUtils#doNormalize .
One might argue that the given example is a misuse of the FileNameUtils.normalize method, and that everyone using it should expect absolute paths, full UNC paths, etc. to be returned by the method.
Furthermore, the vulnerability can only occur due to the lax behavior of java.io.File .
On the other hand, I believe that the JavaDoc of FileNameUtils.normalize suggests to most readers, that this method is exactly what is needed to sanitize file names:
I have done a quick survey of the usages of the method in public GitHub repositories. I have found numerous projects that suffer from the limited path traversal vulnerability that is described here because of this very issue. This includes Webservers, Web-Frameworks, Archive-Extraction-Software, and others.
Preventing path traversal attacks is not trivial, and many people turn to libraries like commons-io to avoid making mistakes when implementing parsing logic on their own. They trust that FileNameUtils will provide them with the most complete, and suitable sanitation logic for file names.
In addition, ".." is not a valid UNC host name according to https://msdn.microsoft.com/de-de/library/gg465305.aspx , so prohibiting it shouldn't result in any major problems.