[commons-compress] 01/04: Make bz2 decompressor robust to selector overflow

[bo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bodewig pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-compress.git

commit bfeaaff800145adac509d069a0c6ebfe22148a89
Author: Joseph Allemandou <jo...@gmail.com>
AuthorDate: Mon Jan 20 21:39:18 2020 +0100

    Make bz2 decompressor robust to selector overflow
    
    As explained in https://gnu.wildebeest.org/blog/mjw/2019/08/02/bzip2-and-the-cve-that-wasnt/
    bz2 has historically been able to decompress wrongly formatted compressed
    files, more precisely files stating more selectors that the algorithm
    actually uses.
    This patch replicates https://sourceware.org/ml/bzip2-devel/2019-q3/msg00007.html
    and provides the same test file as described in the above article (lbzip2_32767.bz2).
---
 .../bzip2/BZip2CompressorInputStream.java          |  13 ++++--
 .../bzip2/BZip2NSelectorsOverflowTest.java         |  46 +++++++++++++++++++++
 src/test/resources/lbzip2_32767.bz2                | Bin 0 -> 4140 bytes
 3 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/apache/commons/compress/compressors/bzip2/BZip2CompressorInputStream.java b/src/main/java/org/apache/commons/compress/compressors/bzip2/BZip2CompressorInputStream.java
index 1a5e2ab..116772c 100644
--- a/src/main/java/org/apache/commons/compress/compressors/bzip2/BZip2CompressorInputStream.java
+++ b/src/main/java/org/apache/commons/compress/compressors/bzip2/BZip2CompressorInputStream.java
@@ -494,17 +494,24 @@ public class BZip2CompressorInputStream extends CompressorInputStream
         final int alphaSize = this.nInUse + 2;
         /* Now the selectors */
         final int nGroups = bsR(bin, 3);
-        final int nSelectors = bsR(bin, 15);
+        int nSelectors = bsR(bin, 15);
         checkBounds(alphaSize, MAX_ALPHA_SIZE + 1, "alphaSize");
         checkBounds(nGroups, N_GROUPS + 1, "nGroups");
-        checkBounds(nSelectors, MAX_SELECTORS + 1, "nSelectors");
+
+        // Don't fail on nSelectors overflowing boundaries but discard the values in overflow
+        // See https://gnu.wildebeest.org/blog/mjw/2019/08/02/bzip2-and-the-cve-that-wasnt/
+        // and https://sourceware.org/ml/bzip2-devel/2019-q3/msg00007.html
 
         for (int i = 0; i < nSelectors; i++) {
             int j = 0;
             while (bsGetBit(bin)) {
                 j++;
             }
-            selectorMtf[i] = (byte) j;
+            if (i < MAX_SELECTORS)
+                selectorMtf[i] = (byte) j;
+        }
+        if (nSelectors > MAX_SELECTORS) {
+            nSelectors = MAX_SELECTORS;
         }
 
         /* Undo the MTF values for the selectors. */
diff --git a/src/test/java/org/apache/commons/compress/compressors/bzip2/BZip2NSelectorsOverflowTest.java b/src/test/java/org/apache/commons/compress/compressors/bzip2/BZip2NSelectorsOverflowTest.java
new file mode 100644
index 0000000..565673c
--- /dev/null
+++ b/src/test/java/org/apache/commons/compress/compressors/bzip2/BZip2NSelectorsOverflowTest.java
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.commons.compress.compressors.bzip2;
+
+import org.apache.commons.compress.AbstractTestCase;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.InputStream;
+
+import static org.junit.Assert.assertEquals;
+
+public class BZip2NSelectorsOverflowTest extends AbstractTestCase {
+
+    /**
+     * See https://sourceware.org/ml/bzip2-devel/2019-q3/msg00007.html
+     */
+    @Test()
+    public void shouldDecompressBlockWithNSelectorOverflow() throws Exception {
+        final File toDecompress = getFile("lbzip2_32767.bz2");
+        final InputStream is = new FileInputStream(toDecompress);
+        final BZip2CompressorInputStream in = new BZip2CompressorInputStream(is);
+        int l = 0;
+        while (in.read() != -1) {
+            l++;
+        }
+        assertEquals(5, l);
+    }
+}
\ No newline at end of file
diff --git a/src/test/resources/lbzip2_32767.bz2 b/src/test/resources/lbzip2_32767.bz2
new file mode 100644
index 0000000..f2755e8
Binary files /dev/null and b/src/test/resources/lbzip2_32767.bz2 differ