Skip to content

Commit 7fadf7e

Browse files
SONARJAVA-6139 Fix FN on S5042 for Apache commons archives (#5524)
1 parent deb4ff3 commit 7fadf7e

4 files changed

Lines changed: 78 additions & 5 deletions

File tree

its/autoscan/src/test/resources/autoscan/autoscan-diff-by-rules.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1994,7 +1994,7 @@
19941994
{
19951995
"ruleKey": "S5042",
19961996
"hasTruePositives": true,
1997-
"falseNegatives": 0,
1997+
"falseNegatives": 8,
19981998
"falsePositives": 0
19991999
},
20002000
{
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"ruleKey": "S5042",
33
"hasTruePositives": true,
4-
"falseNegatives": 0,
4+
"falseNegatives": 8,
55
"falsePositives": 0
6-
}
6+
}

java-checks-test-sources/default/src/main/java/checks/security/ZipEntryCheck.java

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,22 @@
44
import java.io.BufferedOutputStream;
55
import java.io.File;
66
import java.io.FileOutputStream;
7+
import java.io.IOException;
78
import java.io.InputStream;
89
import java.io.OutputStream;
910
import java.util.Enumeration;
11+
import java.util.List;
12+
import java.util.Optional;
1013
import java.util.jar.JarEntry;
1114
import java.util.jar.JarFile;
1215
import java.util.zip.ZipEntry;
1316
import java.util.zip.ZipFile;
1417
import java.util.zip.ZipInputStream;
18+
import org.apache.commons.compress.archivers.sevenz.SevenZArchiveEntry;
19+
import org.apache.commons.compress.archivers.sevenz.SevenZFile;
20+
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
21+
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
22+
import org.apache.commons.compress.archivers.tar.TarFile;
1523

1624
public class ZipEntryCheck {
1725

@@ -93,3 +101,61 @@ public String compliant() throws java.lang.Exception {
93101
}
94102

95103
}
104+
105+
class TarUtilities {
106+
private TarUtilities() {
107+
/* This utility class should not be instantiated */
108+
}
109+
110+
public static List<TarArchiveEntry> getAllEntries(TarFile file) {
111+
return file.getEntries(); // Noncompliant {{Make sure that expanding this archive file is safe here.}}
112+
// ^^^^^^^^^^
113+
}
114+
115+
public static Optional<TarArchiveEntry> getNext(TarArchiveInputStream stream) throws IOException {
116+
return Optional.of(stream.getNextEntry()); // Noncompliant {{Make sure that expanding this archive file is safe here.}}
117+
// ^^^^^^^^^^^^
118+
}
119+
120+
public static long getEntrySize(TarArchiveEntry entry) {
121+
return entry.getSize(); // Noncompliant {{Make sure that expanding this archive file is safe here.}}
122+
// ^^^^^^^
123+
}
124+
}
125+
126+
class SevenZUtilities {
127+
private SevenZUtilities() {
128+
/* This utility class should not be instantiated */
129+
}
130+
131+
public static Iterable<SevenZArchiveEntry> getAllEntries(SevenZFile file) {
132+
return file.getEntries(); // Noncompliant {{Make sure that expanding this archive file is safe here.}}
133+
// ^^^^^^^^^^
134+
}
135+
136+
public static long getEntrySize(SevenZArchiveEntry entry) {
137+
return entry.getSize(); // Noncompliant {{Make sure that expanding this archive file is safe here.}}
138+
// ^^^^^^^
139+
}
140+
}
141+
142+
class ApacheCommonsZipUtilities {
143+
private ApacheCommonsZipUtilities() {
144+
/* This utility class should not be instantiated */
145+
}
146+
147+
public static Enumeration<org.apache.commons.compress.archivers.zip.ZipArchiveEntry> getAllEntries(org.apache.commons.compress.archivers.zip.ZipFile file) {
148+
return file.getEntries(); // Noncompliant {{Make sure that expanding this archive file is safe here.}}
149+
// ^^^^^^^^^^
150+
}
151+
152+
public static Optional<org.apache.commons.compress.archivers.zip.ZipArchiveEntry> getNext(org.apache.commons.compress.archivers.zip.ZipArchiveInputStream stream) throws IOException {
153+
return Optional.of(stream.getNextEntry()); // Noncompliant {{Make sure that expanding this archive file is safe here.}}
154+
// ^^^^^^^^^^^^
155+
}
156+
157+
public static long getEntrySize(org.apache.commons.compress.archivers.zip.ZipArchiveEntry entry) {
158+
return entry.getSize(); // Noncompliant {{Make sure that expanding this archive file is safe here.}}
159+
// ^^^^^^^
160+
}
161+
}

java-checks/src/main/java/org/sonar/java/checks/security/ZipEntryCheck.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,19 @@ public class ZipEntryCheck extends IssuableSubscriptionVisitor {
3838
.addWithoutParametersMatcher()
3939
.build(),
4040
MethodMatchers.create()
41-
.ofSubTypes("java.util.zip.ZipEntry")
41+
.ofSubTypes("org.apache.commons.compress.archivers.tar.TarFile",
42+
"org.apache.commons.compress.archivers.sevenz.SevenZFile",
43+
"org.apache.commons.compress.archivers.zip.ZipFile")
44+
.names("getEntries")
45+
.addWithoutParametersMatcher()
46+
.build(),
47+
MethodMatchers.create()
48+
.ofSubTypes("java.util.zip.ZipEntry", "org.apache.commons.compress.archivers.ArchiveEntry")
4249
.names("getSize")
4350
.addWithoutParametersMatcher()
4451
.build(),
4552
MethodMatchers.create()
46-
.ofSubTypes("java.util.zip.ZipInputStream")
53+
.ofSubTypes("java.util.zip.ZipInputStream", "org.apache.commons.compress.archivers.ArchiveInputStream")
4754
.names("getNextEntry")
4855
.addWithoutParametersMatcher()
4956
.build()

0 commit comments

Comments
 (0)