[commons-compress] branch master updated: COMPRESS-523 be more defensive when skipping over remainder of archive

[bodewig]

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


The following commit(s) were added to refs/heads/master by this push:
     new 2ea0e46  COMPRESS-523 be more defensive when skipping over remainder 
of archive
2ea0e46 is described below

commit 2ea0e4647c6826bea9b04acb104a2954003e8b5c
Author: Stefan Bodewig <bode...@apache.org>
AuthorDate: Sat May 23 21:59:47 2020 +0200

    COMPRESS-523 be more defensive when skipping over remainder of archive
---
 src/changes/changes.xml                            |  3 +++
 .../archivers/zip/ZipArchiveInputStream.java       | 29 +++++++++++++++-------
 .../archivers/zip/ZipArchiveInputStreamTest.java   | 14 +++++++++++
 3 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 41f79f6..e454ea7 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -81,6 +81,9 @@ The <action> type attribute can be add,update,fix,remove.
         IOException rather than a RuntimeException if the zip64 extra
         field of an enty could not be parsed.
       </action>
+      <action issue="COMPRESS-523" type="fix" date="2020-05-23">
+        Improved detection of corrupt ZIP archives in ZipArchiveInputStream.
+      </action>
     </release>
     <release version="1.20" date="2020-02-08"
              description="Release 1.20">
diff --git 
a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java
 
b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java
index 7d15345..5306590 100644
--- 
a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java
+++ 
b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java
@@ -1055,19 +1055,28 @@ public class ZipArchiveInputStream extends 
ArchiveInputStream implements InputSt
         // skip over central directory. One LFH has been read too much
         // already.  The calculation discounts file names and extra
         // data so it will be too short.
-        realSkip((long) entriesRead * CFH_LEN - LFH_LEN);
-        findEocdRecord();
-        realSkip((long) ZipFile.MIN_EOCD_SIZE - WORD /* signature */ - SHORT 
/* comment len */);
-        readFully(shortBuf);
-        // file comment
-        realSkip(ZipShort.getValue(shortBuf));
+        if (entriesRead > 0) {
+            realSkip((long) entriesRead * CFH_LEN - LFH_LEN);
+            boolean foundEocd = findEocdRecord();
+            if (foundEocd) {
+                realSkip((long) ZipFile.MIN_EOCD_SIZE - WORD /* signature */ - 
SHORT /* comment len */);
+                readFully(shortBuf);
+                // file comment
+                final int commentLen = ZipShort.getValue(shortBuf);
+                if (commentLen >= 0) {
+                    realSkip(commentLen);
+                    return;
+                }
+            }
+        }
+        throw new IOException("Truncated ZIP file");
     }
 
     /**
      * Reads forward until the signature of the &quot;End of central
      * directory&quot; record is found.
      */
-    private void findEocdRecord() throws IOException {
+    private boolean findEocdRecord() throws IOException {
         int currentByte = -1;
         boolean skipReadCall = false;
         while (skipReadCall || (currentByte = readOneByte()) > -1) {
@@ -1092,12 +1101,14 @@ public class ZipArchiveInputStream extends 
ArchiveInputStream implements InputSt
                 continue;
             }
             currentByte = readOneByte();
-            if (currentByte == -1
-                || currentByte == ZipArchiveOutputStream.EOCD_SIG[3]) {
+            if (currentByte == -1) {
                 break;
+            } else if (currentByte == ZipArchiveOutputStream.EOCD_SIG[3]) {
+                return true;
             }
             skipReadCall = isFirstByteOfEocdSig(currentByte);
         }
+        return false;
     }
 
     /**
diff --git 
a/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStreamTest.java
 
b/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStreamTest.java
index fb468f1..44ff4b3 100644
--- 
a/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStreamTest.java
+++ 
b/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStreamTest.java
@@ -696,6 +696,20 @@ public class ZipArchiveInputStreamTest {
         });
     }
 
+    @Test
+    /**
+     * @see https://issues.apache.org/jira/browse/COMPRESS-523
+     */
+    public void throwsIfZip64ExtraCouldNotBeUnderstoodY() throws Exception {
+        thrown.expect(IOException.class);
+        thrown.expectMessage("Truncated ZIP file");
+        fuzzingTest(new int[] {
+            0x50, 0x4b, 0x01, 0x02, 0x14, 0x00, 0x14, 0x00, 0x08, 0x00,
+            0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x43, 0xbe, 0x00, 0x00,
+            0x00, 0xb7, 0xe8, 0x07, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00
+        });
+    }
+
     private static byte[] readEntry(ZipArchiveInputStream zip, ZipArchiveEntry 
zae) throws IOException {
         final int len = (int)zae.getSize();
         final byte[] buff = new byte[len];