I'm building a webservice where users can upload zipfiles that get unzipped and saved to our server.
I created the following function to open a file in a specified path:
private File secureOpenFile(String fileName, String directorypath){
return new File(directorypath, fileName);
}
But a security scan tells me that this is not safe, as it has the possibility of path traversal. Giving the arguments ("../../notsafe", "uploadfolder") would allow a malicious attacker to overwrite other files...
However, in the documentation of the File class I found the following: https://docs.oracle.com/javase/7/docs/api/java/io/File.html#File(java.io.File,%20java.lang.String)
Otherwise the parent abstract pathname is taken to denote a directory, and the child pathname string is taken to denote either a directory or a file. If the child pathname string is absolute then it is converted into a relative pathname in a system-dependent way. If parent is the empty abstract pathname then the new File instance is created by converting child into an abstract pathname and resolving the result against a system-dependent default directory. Otherwise each pathname string is converted into an abstract pathname and the child abstract pathname is resolved against the parent.
Which I interpreted as: does not leave the parent folder. Am I correct or not? Is this code safe? And if now: what would be the best way to resolve this security issue?
Yes, it is possible.
...the child abstract pathname is resolved against the parent.
just means the parent path is used as the base when resolving the child path, but if the child path contains .., the result will be outside of the parent.
E.g.
new File("/Users/example/projects/sample/target", "../pom.xml").getCanonicalFile();
results in /Users/example/projects/sample/pom.xml, thus outside of target.
So, in short, yes, you do need to guard against this yourself.