From 8eb10ae1c8658336d43825625a0c1fc9439f796b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Coet?= Date: Wed, 11 Mar 2026 16:31:18 +0100 Subject: [PATCH] Fix FN on S5042 for Apache commons archives --- .../autoscan/autoscan-diff-by-rules.json | 2 +- .../resources/autoscan/diffs/diff_S5042.json | 4 +- .../java/checks/security/ZipEntryCheck.java | 66 +++++++++++++++++++ .../java/checks/security/ZipEntryCheck.java | 11 +++- 4 files changed, 78 insertions(+), 5 deletions(-) diff --git a/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json b/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json index 74dd603242b..85887dbec76 100644 --- a/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json +++ b/its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json @@ -1994,7 +1994,7 @@ { "ruleKey": "S5042", "hasTruePositives": true, - "falseNegatives": 0, + "falseNegatives": 8, "falsePositives": 0 }, { diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5042.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5042.json index 41b5ffa5639..8946310adf2 100644 --- a/its/autoscan/src/test/resources/autoscan/diffs/diff_S5042.json +++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S5042.json @@ -1,6 +1,6 @@ { "ruleKey": "S5042", "hasTruePositives": true, - "falseNegatives": 0, + "falseNegatives": 8, "falsePositives": 0 -} \ No newline at end of file +} diff --git a/java-checks-test-sources/default/src/main/java/checks/security/ZipEntryCheck.java b/java-checks-test-sources/default/src/main/java/checks/security/ZipEntryCheck.java index 2f0471fc4e2..c6b3dce239a 100644 --- a/java-checks-test-sources/default/src/main/java/checks/security/ZipEntryCheck.java +++ b/java-checks-test-sources/default/src/main/java/checks/security/ZipEntryCheck.java @@ -4,14 +4,22 @@ import java.io.BufferedOutputStream; import java.io.File; import java.io.FileOutputStream; +import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.util.Enumeration; +import java.util.List; +import java.util.Optional; import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import java.util.zip.ZipInputStream; +import org.apache.commons.compress.archivers.sevenz.SevenZArchiveEntry; +import org.apache.commons.compress.archivers.sevenz.SevenZFile; +import org.apache.commons.compress.archivers.tar.TarArchiveEntry; +import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; +import org.apache.commons.compress.archivers.tar.TarFile; public class ZipEntryCheck { @@ -93,3 +101,61 @@ public String compliant() throws java.lang.Exception { } } + +class TarUtilities { + private TarUtilities() { + /* This utility class should not be instantiated */ + } + + public static List getAllEntries(TarFile file) { + return file.getEntries(); // Noncompliant {{Make sure that expanding this archive file is safe here.}} + // ^^^^^^^^^^ + } + + public static Optional getNext(TarArchiveInputStream stream) throws IOException { + return Optional.of(stream.getNextEntry()); // Noncompliant {{Make sure that expanding this archive file is safe here.}} + // ^^^^^^^^^^^^ + } + + public static long getEntrySize(TarArchiveEntry entry) { + return entry.getSize(); // Noncompliant {{Make sure that expanding this archive file is safe here.}} + // ^^^^^^^ + } +} + +class SevenZUtilities { + private SevenZUtilities() { + /* This utility class should not be instantiated */ + } + + public static Iterable getAllEntries(SevenZFile file) { + return file.getEntries(); // Noncompliant {{Make sure that expanding this archive file is safe here.}} + // ^^^^^^^^^^ + } + + public static long getEntrySize(SevenZArchiveEntry entry) { + return entry.getSize(); // Noncompliant {{Make sure that expanding this archive file is safe here.}} + // ^^^^^^^ + } +} + +class ApacheCommonsZipUtilities { + private ApacheCommonsZipUtilities() { + /* This utility class should not be instantiated */ + } + + public static Enumeration getAllEntries(org.apache.commons.compress.archivers.zip.ZipFile file) { + return file.getEntries(); // Noncompliant {{Make sure that expanding this archive file is safe here.}} + // ^^^^^^^^^^ + } + + public static Optional getNext(org.apache.commons.compress.archivers.zip.ZipArchiveInputStream stream) throws IOException { + return Optional.of(stream.getNextEntry()); // Noncompliant {{Make sure that expanding this archive file is safe here.}} + // ^^^^^^^^^^^^ + } + + public static long getEntrySize(org.apache.commons.compress.archivers.zip.ZipArchiveEntry entry) { + return entry.getSize(); // Noncompliant {{Make sure that expanding this archive file is safe here.}} + // ^^^^^^^ + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/security/ZipEntryCheck.java b/java-checks/src/main/java/org/sonar/java/checks/security/ZipEntryCheck.java index c47d1abd224..4fb44b47df0 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/security/ZipEntryCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/security/ZipEntryCheck.java @@ -38,12 +38,19 @@ public class ZipEntryCheck extends IssuableSubscriptionVisitor { .addWithoutParametersMatcher() .build(), MethodMatchers.create() - .ofSubTypes("java.util.zip.ZipEntry") + .ofSubTypes("org.apache.commons.compress.archivers.tar.TarFile", + "org.apache.commons.compress.archivers.sevenz.SevenZFile", + "org.apache.commons.compress.archivers.zip.ZipFile") + .names("getEntries") + .addWithoutParametersMatcher() + .build(), + MethodMatchers.create() + .ofSubTypes("java.util.zip.ZipEntry", "org.apache.commons.compress.archivers.ArchiveEntry") .names("getSize") .addWithoutParametersMatcher() .build(), MethodMatchers.create() - .ofSubTypes("java.util.zip.ZipInputStream") + .ofSubTypes("java.util.zip.ZipInputStream", "org.apache.commons.compress.archivers.ArchiveInputStream") .names("getNextEntry") .addWithoutParametersMatcher() .build()