change apache commons sink

Author: am0o0

java/ql/src/experimental/semmle/code/java/security/DecompressionBomb.qll

@@ -156,6 +156,30 @@ module ApacheCommons {
         this.getCallee().hasName(["read", "readNBytes", "readAllBytes"])
       }
     }
+
+    /**
+     * Gets `n1` and `n2` which `GzipCompressorInputStream n2 = new GzipCompressorInputStream(n1)`
+     */
+    private class CompressorsAndArchiversAdditionalTaintStep extends DecompressionBomb::AdditionalStep
+    {
+      override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+        exists(Call call |
+          // Constructors
+          call.getCallee().getDeclaringType() instanceof TypeCompressors and
+          call.getArgument(0) = n1.asExpr() and
+          call = n2.asExpr()
+        )
+      }
+    }
+
+    predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+      exists(Call call |
+        // Constructors
+        call.getCallee().getDeclaringType() instanceof TypeCompressors and
+        call.getArgument(0) = n1.asExpr() and
+        call = n2.asExpr()
+      )
+    }
   }
 
   /**
@@ -191,6 +215,25 @@ module ApacheCommons {
         this.getCallee().hasName(["read", "readNBytes", "readAllBytes"])
       }
     }
+
+    /**
+     * Gets `n1` and `n2` which `CompressorInputStream n2 = new CompressorStreamFactory().createCompressorInputStream(n1)`
+     * or `ArchiveInputStream n2 = new ArchiveStreamFactory().createArchiveInputStream(n1)` or
+     * `n1.read(n2)`,
+     * second one is added because of sanitizer, we want to compare return value of each `read` or similar method
+     * that whether there is a flow to a comparison between total read of decompressed stream and a constant value
+     */
+    private class CompressorsAndArchiversAdditionalTaintStep extends DecompressionBomb::AdditionalStep
+    {
+      override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+        exists(Call call |
+          // Constructors
+          call.getCallee().getDeclaringType() instanceof TypeArchivers and
+          call.getArgument(0) = n1.asExpr() and
+          call = n2.asExpr()
+        )
+      }
+    }
   }
 
   /**

java/ql/test/experimental/query-tests/security/CWE-522-DecompressionBombs/Bombs.java

@@ -38,7 +38,6 @@ public void sendUserFileGood2(Socket sock) throws IOException {
         // SnappyZip
         SnappyHandler.SnappyZipInputStream(remoteFile);
         // apache Commons
-        CommonsCompressHandler.commonsCompressArchiveInputStream2(remoteFile);
         CommonsCompressHandler.commonsCompressorInputStream(remoteFile);
         try {
             CommonsCompressHandler.commonsCompressArchiveInputStream(remoteFile);

java/ql/test/experimental/query-tests/security/CWE-522-DecompressionBombs/CommonsCompressHandler.java

@@ -34,36 +34,45 @@
 
 public class CommonsCompressHandler {
 
-    static void commonsCompressArchiveInputStream(InputStream inputStream) throws ArchiveException {
-        new ArArchiveInputStream(inputStream); // $ hasTaintFlow="inputStream"
-        new ArjArchiveInputStream(inputStream); // $ hasTaintFlow="inputStream"
-        new CpioArchiveInputStream(inputStream); // $ hasTaintFlow="inputStream"
-        new JarArchiveInputStream(inputStream); // $ hasTaintFlow="inputStream"
-        new ZipArchiveInputStream(inputStream); // $ hasTaintFlow="inputStream"
-    }
-
     public static void commonsCompressorInputStream(InputStream inputStream) throws IOException {
         BufferedInputStream in = new BufferedInputStream(inputStream);
         OutputStream out = Files.newOutputStream(Path.of("tmpfile"));
-        GzipCompressorInputStream gzIn = new GzipCompressorInputStream(in); // $ hasTaintFlow="in"
-        // for testing
-        new BrotliCompressorInputStream(in); // $ hasTaintFlow="in"
-        new BZip2CompressorInputStream(in); // $ hasTaintFlow="in"
-        new DeflateCompressorInputStream(in); // $ hasTaintFlow="in"
-        new Deflate64CompressorInputStream(in); // $ hasTaintFlow="in"
-        new BlockLZ4CompressorInputStream(in); // $ hasTaintFlow="in"
-        new LZMACompressorInputStream(in); // $ hasTaintFlow="in"
-        new Pack200CompressorInputStream(in); // $ hasTaintFlow="in"
-        new SnappyCompressorInputStream(in); // $ hasTaintFlow="in"
-        new XZCompressorInputStream(in); // $ hasTaintFlow="in"
-        new ZCompressorInputStream(in); // $ hasTaintFlow="in"
-        new ZstdCompressorInputStream(in); // $ hasTaintFlow="in"
+        GzipCompressorInputStream gzIn = new GzipCompressorInputStream(in);
+        // Also, the `new GzipCompressorInputStream(in)` can be the following:
+        // new BrotliCompressorInputStream(in);
+        // new BZip2CompressorInputStream(in);
+        // new DeflateCompressorInputStream(in);
+        // new Deflate64CompressorInputStream(in);
+        // new BlockLZ4CompressorInputStream(in);
+        // new LZMACompressorInputStream(in);
+        // new Pack200CompressorInputStream(in);
+        // new SnappyCompressorInputStream(in);
+        // new XZCompressorInputStream(in);
+        // new ZCompressorInputStream(in);
+        // new ZstdCompressorInputStream(in);
+
+        int buffersize = 4096;
+        final byte[] buffer = new byte[buffersize];
+        int n = 0;
+        while (-1 != (n = gzIn.read(buffer))) {  // $ hasTaintFlow="gzIn"
+            out.write(buffer, 0, n);
+        }
+        out.close();
+        gzIn.close();
     }
 
-    static void commonsCompressArchiveInputStream2(InputStream inputStream) {
+    static void commonsCompressArchiveInputStream(InputStream inputStream) {
         byte[] readBuffer = new byte[4096];
-        try (org.apache.commons.compress.archivers.zip.ZipArchiveInputStream zipInputStream =
-                     new org.apache.commons.compress.archivers.zip.ZipArchiveInputStream(inputStream)) { // $ hasTaintFlow="inputStream"
+
+        // Also, the `new ZipArchiveInputStream(inputStream)` can be the following:
+        // new ArArchiveInputStream(inputStream);
+        // new ArjArchiveInputStream(inputStream);
+        // new CpioArchiveInputStream(inputStream);
+        // new JarArchiveInputStream(inputStream);
+        // new ZipArchiveInputStream(inputStream);
+
+        try (ZipArchiveInputStream zipInputStream =
+                     new ZipArchiveInputStream(inputStream)) { 
             ArchiveEntry entry = null;
             while ((entry = zipInputStream.getNextEntry()) != null) {
                 if (!zipInputStream.canReadEntryData(entry)) {
@@ -72,7 +81,7 @@ static void commonsCompressArchiveInputStream2(InputStream inputStream) {
                 File f = new File("tmpfile");
                 try (OutputStream outputStream = new FileOutputStream(f)) {
                     int readLen;
-                    while ((readLen = zipInputStream.read(readBuffer)) != -1) {
+                    while ((readLen = zipInputStream.read(readBuffer)) != -1) {  // $ hasTaintFlow="zipInputStream"
                         outputStream.write(readBuffer, 0, readLen);
                     }
                 }