You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Sravan Putluru (Jira)" <ji...@apache.org> on 2020/03/11 17:23:00 UTC
[jira] [Commented] (IO-556) Unexpected behavior of
FileNameUtils.normalize may lead to limited path traversal vulnerabilies
[ https://issues.apache.org/jira/browse/IO-556?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17057237#comment-17057237 ]
Sravan Putluru commented on IO-556:
-----------------------------------
Project uses *normalize()* to generated file path based on windows\linux but in VeraCode security can report method used line detected as Directory Traversal T issue as medium flaws.
Common.io 2.6 API Unexpected behavior with normalize(String s) method is not performing validations on path input. "../ " is allowing but return as Null if the input type is some thing like "{color:#FF0000}*../../*{color}". with the below lines of code checks can be remove path DT vulnerabilities issue. Could somebody please give solution.
Veracode report result Directiry Travesal medium flaws detected need to fix.
fileName = "../../etc/passwd";
fileName = FilenameUtils.normalize(fileName); // still holds the same value ("//../foo")
if (fileName != null) {
// file creation path eg: drivec\root\06-03-2020\folder\test
}
else {
throw new CustomerException("Invalid path creation found");
}
> Unexpected behavior of FileNameUtils.normalize may lead to limited path traversal vulnerabilies
> -----------------------------------------------------------------------------------------------
>
> Key: IO-556
> URL: https://issues.apache.org/jira/browse/IO-556
> Project: Commons IO
> Issue Type: Bug
> Components: Utilities
> Affects Versions: 2.2, 2.3, 2.4, 2.5, 2.6
> Environment: all
> Reporter: Lukas Euler
> Priority: Major
> Labels: security, security-issue
>
> I sent this report in an Email to security@apache.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.
> h5. Reproduction
> {Code}
> FileNameUtils.normalize("//../foo"); // returns "//../foo" or "\\\\..\\foo", based on java.io.File.separatorChar
> FileNameUtils.normalize("\\\\..\\foo"); // returns "//../foo" or "\\\\..\\foo", based on java.io.File.separatorChar
> {Code}
> h5. Possible impact (example)
> 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:
> {Code}
> String fileName = "//../foo"; // actually user-supplied (e.g. from multipart-POST request)
> fileName = FileNameUtils.normalize(fileName); // still holds the same value ("//../foo")
>
> if (fileName != null) {
> File newFile = new File("/base/uploads", fileName); // java.io.File treats double forward slashes as singles
> // newFile now points to "/base/uploads//../foo"
> newFile = newFile.getCanonicalFile(); // newFile now points to "/base/foo", which should be inaccessible
> // Write contents to newFile...
> } else {
> // Assume malicious activity, handle error
> }
> {Code}
> h5. Relevant code locations
> * {{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}} .
> h5. Discussion
> 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:
> {noformat}
> //-----------------------------------------------------------------------
> /**
> * Normalizes a path, removing double and single dot path steps,
> * and removing any final directory separator.
> * <p>
> * This method normalizes a path to a standard format.
> * The input may contain separators in either Unix or Windows format.
> * The output will contain separators in the format of the system.
> * <p>
> * A trailing slash will be removed.
> * A double slash will be merged to a single slash (but UNC names are handled).
> * A single dot path segment will be removed.
> * A double dot will cause that path segment and the one before to be removed.
> * If the double dot has no parent path segment to work with, {@code null}
> * is returned.
> * <p>
> * The output will be the same on both Unix and Windows except
> * for the separator character.
> * <pre>
> * /foo// --> /foo
> * /foo/./ --> /foo
> * /foo/../bar --> /bar
> * /foo/../bar/ --> /bar
> * /foo/../bar/../baz --> /baz
> * //foo//./bar --> /foo/bar
> * /../ --> null
> * ../foo --> null
> * foo/bar/.. --> foo
> * foo/../../bar --> null
> * foo/../bar --> bar
> * //server/foo/../bar --> //server/bar
> * //server/../bar --> null
> * C:\foo\..\bar --> C:\bar
> * C:\..\bar --> null
> * ~/foo/../bar/ --> ~/bar
> * ~/../bar --> null
> * </pre>
> * (Note the file separator returned will be correct for Windows/Unix)
> *
> * @param filename the filename to normalize, null returns null
> * @return the normalized filename, or null if invalid. Null bytes inside string will be removed
> */
> {noformat}
> 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.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)