LGTM.com - false positive - Zip Slip when guard `FileNameUtils#normalize` is used

[JLLeitschuh]

Description of the false positive

org.apache.commons.io.FileNameUtils#normalize should be a valid guard against Zip-Slip.

https://lgtm.com/projects/g/kubernetes-client/java/snapshot/bf96dd988546f64a04a39f53ef5b6c98bcc90475/files/util/src/main/java/io/kubernetes/client/Copy.java?sort=name&dir=ASC&mode=heatmap#L191

https://commons.apache.org/proper/commons-io/apidocs/org/apache/commons/io/FilenameUtils.html#normalize-java.lang.String-

[hvitved]

@github/codeql-java one for you.

[Marcono1234]

Depending on how sophisticated the guard detection should be, FileNameUtils#normalize might not on its own be enough to sanitize the path. In the code example you have shown, the code uses java.io.File to construct the path and therefore seems to be safe.

However, if code uses java.nio.file.Path, then FileNameUtils#normalize would not suffice for sanitization because Path.resolve behaves differently for absolute paths and paths with a different root component. For example on Windows:

jshell> Path.of("C:\\safe").resolve("\\not-safe")
==> C:\not-safe

jshell> Path.of("C:\\safe").resolve("D:not-safe")
==> D:not-safe

---

Note that I am not a member of this project.

[atorralba]

Aside from what @Marcono1234 pointed out, depending on how FilenameUtils.normalize itself is used, it won't be a proper sanitizer either. For example:

FilenameUtils.normalize("/safe/" + userInput()); // userInput() = "../unsafe/"

would return /unsafe/.

So apparently FilenameUtils.normalize would only be a valid sanitizer if it's used directly on the user input AND the result is prefixed with a trusted path (i.e. not used as an absolute path), and Path.resolve is NOT used to do so.