Stop searching after finding acceptable central directory, even if it…

… contains inconsistencies.
nih-at · Jul 19, 2024 · ea70497 · ea70497
1 parent 5f55ecd
commit ea70497
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 56 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,6 @@
 # X.X [Unreleased]
 
+* Stop searching after finding acceptable central directory, even if it contains inconsistencies.
 * Only write Zip64 EOCD if fields don't fit in normal EOCD. Previously libzip also wrote it when any directory entry required Zip64.
 * Allow bytes from 0x00-0x1F as UTF-8.
 * Add new error code `ZIP_ER_TRUNCATED_ZIP` for files that start with a valid local header signature.

diff --git a/lib/zip_open.c b/lib/zip_open.c
@@ -31,7 +31,6 @@
   IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */
 
-
 #include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -46,10 +45,10 @@ static void zip_check_torrentzip(zip_t *za, const zip_cdir_t *cdir);
 static zip_cdir_t *_zip_find_central_dir(zip_t *za, zip_uint64_t len);
 static exists_t _zip_file_exists(zip_source_t *src, zip_error_t *error);
 static int _zip_headercomp(const zip_dirent_t *, const zip_dirent_t *);
-static const unsigned char *_zip_memmem(const unsigned char *, size_t, const unsigned char *, size_t);
 static zip_cdir_t *_zip_read_cdir(zip_t *za, zip_buffer_t *buffer, zip_uint64_t buf_offset, zip_error_t *error);
 static zip_cdir_t *_zip_read_eocd(zip_buffer_t *buffer, zip_uint64_t buf_offset, unsigned int flags, zip_error_t *error);
 static zip_cdir_t *_zip_read_eocd64(zip_source_t *src, zip_buffer_t *buffer, zip_uint64_t buf_offset, unsigned int flags, zip_error_t *error);
+static const unsigned char *find_eocd(zip_buffer_t *buffer, const unsigned char *last);
 
 
 ZIP_EXTERN zip_t *
@@ -596,12 +595,10 @@ _zip_file_exists(zip_source_t *src, zip_error_t *error) {
 
 static zip_cdir_t *
 _zip_find_central_dir(zip_t *za, zip_uint64_t len) {
-    zip_cdir_t *cdir, *cdirnew;
+    zip_cdir_t *cdir;
     const zip_uint8_t *match;
     zip_int64_t buf_offset;
     zip_uint64_t buflen;
-    zip_int64_t a;
-    zip_int64_t best;
     zip_error_t error;
     zip_buffer_t *buffer;
 
@@ -628,84 +625,58 @@ _zip_find_central_dir(zip_t *za, zip_uint64_t len) {
         return NULL;
     }
 
-    best = -1;
     cdir = NULL;
     if (buflen >= CDBUFSIZE) {
         /* EOCD64 locator is before EOCD, so leave place for it */
         _zip_buffer_set_offset(buffer, EOCD64LOCLEN);
     }
     zip_error_set(&error, ZIP_ER_NOZIP, 0);
 
-    match = _zip_buffer_get(buffer, 0);
-    /* The size of buffer never greater than CDBUFSIZE. */
-    while (_zip_buffer_left(buffer) >= EOCDLEN && (match = _zip_memmem(match, (size_t)_zip_buffer_left(buffer) - (EOCDLEN - 4), (const unsigned char *)EOCD_MAGIC, 4)) != NULL) {
+    match = NULL;
+    while ((match = find_eocd(buffer, match)) != NULL) {
         _zip_buffer_set_offset(buffer, (zip_uint64_t)(match - _zip_buffer_data(buffer)));
-        if ((cdirnew = _zip_read_cdir(za, buffer, (zip_uint64_t)buf_offset, &error)) != NULL) {
-            if (cdir) {
-                if (best <= 0) {
-                    best = _zip_checkcons(za, cdir, &error);
-                }
-
-                a = _zip_checkcons(za, cdirnew, &error);
-                if (best < a) {
-                    _zip_cdir_free(cdir);
-                    cdir = cdirnew;
-                    best = a;
-                }
-                else {
-                    _zip_cdir_free(cdirnew);
-                }
-            }
-            else {
-                cdir = cdirnew;
-                if (za->open_flags & ZIP_CHECKCONS)
-                    best = _zip_checkcons(za, cdir, &error);
-                else {
-                    best = 0;
-                }
+        if ((cdir = _zip_read_cdir(za, buffer, (zip_uint64_t)buf_offset, &error)) != NULL) {
+            if ((za->open_flags & ZIP_CHECKCONS) && _zip_checkcons(za, cdir, &error) < 0) {
+                _zip_error_copy(&za->error, &error);
+                _zip_buffer_free(buffer);
+                _zip_cdir_free(cdir);
+                return NULL;
             }
-            cdirnew = NULL;
+            break;
         }
 
-        match++;
-        _zip_buffer_set_offset(buffer, (zip_uint64_t)(match - _zip_buffer_data(buffer)));
+        if (match == _zip_buffer_data(buffer)) {
+            break;
+        }
+        match -= 1;
     }
 
     _zip_buffer_free(buffer);
 
-    if (best < 0) {
+    if (cdir == NULL) {
         _zip_error_copy(&za->error, &error);
-        _zip_cdir_free(cdir);
-        return NULL;
     }
-
     return cdir;
 }
 
 
-static const unsigned char *
-_zip_memmem(const unsigned char *big, size_t biglen, const unsigned char *little, size_t littlelen) {
+static const unsigned char *find_eocd(zip_buffer_t *buffer, const unsigned char *last) {
+    const unsigned char *data = _zip_buffer_data(buffer);
     const unsigned char *p;
 
-    if (littlelen == 0) {
-        return big;
-    }
-
-    if (biglen < littlelen) {
-        return NULL;
+    if (last == NULL) {
+        last = data + _zip_buffer_size(buffer) - MAGIC_LEN;
     }
 
-    p = big;
-    while (true) {
-        p = (const unsigned char *)memchr(p, little[0], biglen - (littlelen - 1) - (size_t)(p - big));
-        if (p == NULL) {
-            return NULL;
-        }
-        if (memcmp(p + 1, little + 1, littlelen - 1) == 0) {
-            return p;
+    for (p = last; p >= data; p -= 1) {
+        if (*p == EOCD_MAGIC[0]) {
+            if (memcmp(p, EOCD_MAGIC, MAGIC_LEN) == 0) {
+                return p;
+            }
         }
-        p += 1;
     }
+
+    return NULL;
 }
 
 

diff --git a/lib/zipint.h b/lib/zipint.h
@@ -56,6 +56,7 @@
 #define DATADES_MAGIC "PK\7\10"
 #define EOCD64LOC_MAGIC "PK\6\7"
 #define EOCD64_MAGIC "PK\6\6"
+#define MAGIC_LEN 4
 #define CDENTRYSIZE 46u
 #define LENTRYSIZE 30
 #define MAXCOMLEN 65536