Dave562
(Dave562)
September 7, 2023, 11:32am
1
Make sure to read this post before raising a thread here:
Hey SonarSource Community!
False-positives happen , as do false-negatives, and we’re eager to fix them. We are thrilled when our users report problems, so we can make our products better.
What is a false-positive (FP)?
A false-positive is when an issue is raised unexpectedly on code that should not trigger an issue, or where the suggested action doesn’t make any sense for the code.
What is a false-negative (FN)?
A false-negative is when an issue should be raised on a piece of code, but isn’t…
Then tell us:
What language is this for?
Which rule?
Why do you believe it’s a false-positive?
It is prevented by checking normalized path with targetDirectory path as described here
Are you using
SonarQube - Enterprise Edition Version 10.2 (build 77647)
How can we reproduce the problem?
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Path;
import java.util.Enumeration;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
public class Example {
private static final String targetDirectory = "/example/directory/";
public void extractEntry(ZipFile zipFile) throws IOException {
Enumeration<? extends ZipEntry> entries = zipFile.entries();
while (entries.hasMoreElements()) {
ZipEntry entry = entries.nextElement();
String name = entry.getName();
String dest = targetDirectory + name;
if (!Path.of(dest).normalize().startsWith(Path.of(targetDirectory).normalize())) {
throw new IOException("Entry is outside of the target dir.");
}
try (InputStream zis = zipFile.getInputStream(entry);
FileOutputStream fos = new FileOutputStream(dest)) {
byte[] readBuffer = new byte[4096];
int bytesIn;
while ((bytesIn = zis.read(readBuffer)) != -1) {
fos.write(readBuffer, 0, bytesIn);
}
fos.flush();
}
}
}
}
Dave562
(Dave562)
September 14, 2023, 5:06am
6
Answered here by @Pierre-Loup_Tristant :
This answer was ment to be posted here [javasecurity:S6096] Zip slip reported when prevented using Java NIO - #4
Hi @Dave562
Thanks again for your feedback and for the code example you shared. I confirmed that I could reproduce this FP. It’s caused by a known limitation in our engine.
I can also confirm that the code you shared is not vulnerable to ZipSlip and you can safely change the issue status to Resolved (False-Positive).
Pierre-Loup
Hi Dave562,
Yes, this is the correct answer. I somehow replied to the wrong thread, sorry about that.
Hello @Pierre-Loup_Tristant ,
thanks for confirmation. I just linked your answer regarding FP to the other issue, to which I believe your answer belong. Just out of curiosity, is the known limitation you are refering to going to be lifted at some point? And is it also affecting other rules, like for example is it in general ignored as using IO when using NIO?
The limitation is related to validators of path-traversal-like vulnerability (S2083 and S6096). It doesn’t affect other rules. The “universal” way to protect against path traversal happens in two steps: compute the normalized/canonical path, then make sure this path is not located outside of the destination folder. As of today the engine does not correctly support this two-step validation and it leads to false-positive. I don’t know when we will be able to overcome this limitation.
Pierre-Loup
Dave562
(Dave562)
September 14, 2023, 7:48am
9
Hello @Pierre-Loup_Tristant ,
thanks for explanation, I don’t have access to linked Jira, but I understand the limitation now.