SonarQube Data Center Edition Version 9.9.3 (build 79811)
Legacy code incremental improvement on a very strict and restrict corporate setup
Junit running in Java 21
Hello everyone, first I want to thank you for your time, your patience and hopefully for your help.
For the rule javasecurity:S2083 the Compliant solution:
public class ExampleController
{
static private String targetDirectory = “/path/to/target/directory/”;
@GetMapping(value = "/delete")
public void delete(@RequestParam("filename") String filename) throws IOException {
File file = new File(targetDirectory + filename);
String canonicalDestinationPath = file.getCanonicalPath();
if (!canonicalDestinationPath.startsWith(targetDirectory)) {
throw new IOException("Entry is outside of the target directory");
}
file.delete();
}
}
The issue, is that if /path is a symbolic link to /private/path (happened in my local development Mac OS machine) the getCanonicalPath will resolve it to /private/path and will throw the IOException(“Entry is outside of the target directory”).
One could handle it as a special case removing /private if on a Mac OS… and assume at server setup no symbolic link is involved on target… or maybe use Path.resolve instead, it appears in the rule on the Pitfalls section and seems to be discouraged…
Any suggestion on how to handle this in a secure, robust and elegant way?
May I also suggest adding something on this lines to javasecurity:S2083 description as a possible improvement.
var file = Files.createTempFile("Prefix_", ".tmp");
Since in macOs this file is a folder with a symbolic link (/private/xxx), I workaround using a different folder, the current work folder, System.getProperty(“user.dir”), and luckily this one has no symbolic link, code like:
var fileToDownload = Files.createTempFile(Path.of(System.getProperty("user.dir")),"Prefix_", ".tmp");
And hopping there are no symbolic links on the folder involved on server side version of the application (risk of breaking normal functionality).
Hope this rambling might be useful for anyone, and do point out if any error in my logic, have a nice day.
I understand that the problem occurred to you because on MacOS /tmp, /etc, and /var are symbolic links to /private/tmp, etc.
I would argue that the rule still behaves in a correct and wanted way. If it was possible to access files outside of the target directory via symbolic links, an attacker could abuse this behavior.
If your application should only be deployed on MacOS, you can change targetDirectory to “/private/…” (I think this is also what you mentioned in your post).
If you want to be able to host your application also on Linux and you want to make use of a folder in /tmp, /etc, or /var (which is probably not a good idea for production, see: S5443) you can extend the check:
if (!canonicalDestinationPath.startsWith(targetDirectory)
&& !canonicalDestinationPath.startsWith("/private" +targetDirectory)) {
throw new IOException("Entry is outside of the target directory");
}
This can also be applied if you have any symlinks that point to other folders. However, it is important to only allow access to safe folders!
We will consider if it makes sense to add some information about symbolic links or the MacOS behavior in the rule description of javasecurity:S2083.
Thanks again for your post, I hope I could help you!
Just a note, on highly siloed work environments like mine, I do not have visibility to how the application is actually configured and deployed on the server, so the issue with symlinks might be more then just MacOS /private symlinks depending on server side specifics like cluster and other possible filesystem symlinks. So I will advise the Ops team to test specifically the functionality that might be impacted by this before considering the pre-prod test concluded. So the note could exemplify with MacOS, but it is more general if my memory server me well, the case where I saw symlinks as as mechanism to have cluster filesystem in the past was on an old Solaris or was it a AIX, not sure.