[ https://issues.apache.org/jira/browse/IO-556?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17412773#comment-17412773 ]
Julius Davies edited comment on IO-556 at 9/9/21, 6:46 PM: ----------------------------------------------------------- [~jameshowe] - goes back to 1.1. FilenameUtils not present in 1.0. {code:java} java -cp commons-io-1.1.jar:. T '//../foo' //../foo {code} {code:java} java -cp commons-io-2.6.jar:. T '//../foo' //../foo {code} {code:java} java -cp commons-io-2.7.jar:. T '//../foo' null {code} (Based on a T.java like this): {code:java} public class T { public class T { public static void main(String[] args) throws Exception { System.out.println( org.apache.commons.io.FilenameUtils.normalize(args[0])); } } {code} was (Author: juliusdavies): [~jameshowe] - goes back to 1.1. FilenameUtils not present in 1.0. {code:java} java -cp commons-io-1.1.jar:. T '//../foo' //../foo {code} {code:java} java -cp commons-io-2.6.jar:. T '//../foo' //../foo {code} {code:java} java -cp commons-io-2.7.jar:. T '//../foo' null {code} (Based on a T.java like this): {code:java} public class T { public class T { public static void main(String[] args) throws Exception { System.out.println( org.apache.commons.io.FilenameUtils.normalize(args[0])); } } {code} > 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 secur...@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)