Rewrite Eocdr64Locator search algorithm

3 files changed, 93 insertions, 66 deletions

diff --git a/src/zip/driver.rs b/src/zip/driver.rs
index 7758479..0502c85 100644
--- a/src/zip/driver.rs
+++ b/src/zip/driver.rs
@@ -79,7 +79,7 @@ impl<Io: Read + Seek> ArchiveRead for Zip<Io> {
         // Search eocdr
         let limit = 65557.min(io.seek(SeekFrom::End(0))?) as i64;
         let start = io.seek(SeekFrom::End(-limit))?;
-        let pos = start
+        let eocdr_pos = start
             + io.read_vec(
                 (limit as usize)
                     .checked_sub(18)
@@ -90,46 +90,50 @@ impl<Io: Read + Seek> ArchiveRead for Zip<Io> {
             .ok_or(ZipError::StructNotFound("Eocdr"))? as u64;
 
         // Read eocdr
-        io.seek(SeekFrom::Start(pos + 4))?;
+        io.seek(SeekFrom::Start(eocdr_pos + 4))?;
         let buf = io.read_arr::<18>()?;
         let eocdr: Eocdr = deserialize(&buf).unwrap();
         let comment = String::from_cp437(io.read_vec(eocdr.comment_len as usize)?);
 
+        let mut cd_pointer = eocdr.cd_pointer as u64;
+        let mut cd_size = eocdr.cd_size as u64;
+        let mut cd_records = eocdr.cd_records as u64;
+
         // Try to find eocdr64locator
-        io.seek(SeekFrom::Start(pos.saturating_sub(20)))?;
-        let buf = io.read_arr::<20>()?;
-        let (cd_pointer, cd_size, cd_records) = if buf[..4] == EOCDR64_LOCATOR_SIGNATURE {
-            // Locator found
-            let eocdr64locator: Eocdr64Locator = deserialize(&buf[4..]).unwrap();
-
-            io.seek(SeekFrom::Start(eocdr64locator.eocdr64_pointer))?;
-            if io.read_arr()? != EOCDR64_SIGNATURE {
-                return Err(ZipError::InvalidSignature("Eocdr64"));
-            }
+        if eocdr_pos >= 20 {
+            io.seek(SeekFrom::Start(eocdr_pos - 20))?;
+            let buf = io.read_arr::<20>()?;
 
-            let eocdr64: Eocdr64 = deserialize(&io.read_arr::<52>()?).unwrap();
-            if eocdr64.cd_pointer + eocdr64.cd_size > eocdr64locator.eocdr64_pointer {
-                return Err(ZipError::Overlapping(
-                    "Central directory records",
-                    "Zip64 end of central directory record",
-                ));
-            }
+            if buf[..4] == EOCDR64_LOCATOR_SIGNATURE {
+                let locator: Eocdr64Locator = deserialize(&buf[4..]).unwrap();
+                io.seek(SeekFrom::Start(locator.eocdr64_pointer))?;
 
-            (eocdr64.cd_pointer, eocdr64.cd_size, eocdr64.cd_records)
-        } else {
-            if (eocdr.cd_pointer + eocdr.cd_size) as u64 > pos {
-                return Err(ZipError::Overlapping(
-                    "Central directory records",
-                    "End of central directory record",
-                ));
-            }
+                if io.read_arr()? != EOCDR64_SIGNATURE {
+                    return Err(ZipError::InvalidSignature("Eocdr64"));
+                }
+
+                if locator.eocdr64_pointer + 76 > eocdr_pos {
+                    return Err(ZipError::Overlapping("Eocdr64", "Eocdr64Locator"));
+                }
 
-            (
-                eocdr.cd_pointer as u64,
-                eocdr.cd_size as u64,
-                eocdr.cd_records as u64,
-            )
-        };
+                let eocdr64: Eocdr64 = deserialize(&io.read_arr::<54>()?).unwrap();
+                if locator.eocdr64_pointer + eocdr64.eocdr64_size + 32 > eocdr_pos {
+                    return Err(ZipError::Overlapping("Eocdr64", "Eocdr64Locator"));
+                }
+
+                cd_pointer = eocdr64.cd_pointer;
+                cd_size = eocdr64.cd_size;
+                cd_records = eocdr64.cd_records;
+
+                if cd_pointer + cd_size > locator.eocdr64_pointer {
+                    return Err(ZipError::Overlapping("Cdr", "Eocdr64"));
+                }
+            } else if cd_pointer + cd_size > eocdr_pos {
+                return Err(ZipError::Overlapping("Cdr", "Eocdr"));
+            }
+        } else if cd_pointer + cd_size > eocdr_pos {
+            return Err(ZipError::Overlapping("Cdr", "Eocdr"));
+        }
 
         // Read cd records
         let mut indexes = Map::with_capacity(cd_records as usize);
diff --git a/src/zip/error.rs b/src/zip/error.rs
index fbc2ba1..40d45fe 100644
--- a/src/zip/error.rs
+++ b/src/zip/error.rs
@@ -31,10 +31,10 @@ impl From<IoError> for ZipError {
 impl PartialEq for ZipError {
     fn eq(&self, other: &Self) -> bool {
         match (self, other) {
-            (Self::Io(l0), Self::Io(r0)) => l0.kind() == r0.kind(),
-            (Self::StructNotFound(l0), Self::StructNotFound(r0)) => l0 == r0,
-            (Self::InvalidSignature(l0), Self::InvalidSignature(r0)) => l0 == r0,
-            (Self::InvalidField(l0), Self::InvalidField(r0)) => l0 == r0,
+            (Self::Io(l), Self::Io(r)) => l.kind() == r.kind(),
+            (Self::StructNotFound(l), Self::StructNotFound(r)) => l == r,
+            (Self::InvalidSignature(l), Self::InvalidSignature(r)) => l == r,
+            (Self::InvalidField(l), Self::InvalidField(r)) => l == r,
             (Self::Overlapping(l0, l1), Self::Overlapping(r0, r1)) => l0 == r0 && l1 == r1,
             _ => core::mem::discriminant(self) == core::mem::discriminant(other),
         }
diff --git a/tests/zip.rs b/tests/zip.rs
index 1422bb3..e00789d 100644
--- a/tests/zip.rs
+++ b/tests/zip.rs
@@ -90,11 +90,17 @@ fn test_zip_weak() {
     }
 }
 
-const EMPTY: Cursor<&[u8]> = Cursor::new(b"PK\x05\x06\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0");
-
 #[test]
 fn test_zip() {
-    assert_eq!(Archive::<Zip<_>>::read(EMPTY).unwrap().len(), 0);
+    assert_eq!(
+        Archive::<Zip<_>>::read(Cursor::new(&[
+            0x50, 0x4b, 0x05, 0x06, 0, 0, 0, 0, 0, //
+            0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr
+        ]))
+        .unwrap()
+        .len(),
+        0
+    );
 
     let mut archive = Archive::<Zip>::read_from_file("tests/files/archive.zip").unwrap();
 
@@ -158,32 +164,49 @@ fn test_zip() {
     }
 }
 
-const NOT_FOUND: Cursor<&[u8]> = Cursor::new(b"");
-const INVALID: Cursor<&[u8]> = Cursor::new(
-    b"PK\x06\x07\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0PK\x05\x06\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0",
-);
-const OVERLAP: Cursor<&[u8]> = Cursor::new(b"PK\x05\x06\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0");
-const OVERLAP64: Cursor<&[u8]> = Cursor::new(b"PK\x06\x06\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0PK\x06\x07\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0PK\x05\x06\0\0\0\0\0\0\0\0\x01\0\0\0\0\0\0\0\0\0");
-
 #[test]
 fn test_bad_zip() {
-    assert!(
-        Archive::<Zip<_>>::read(NOT_FOUND).is_err_and(|e| e == ZipError::StructNotFound("Eocdr"))
-    );
-
-    assert!(
-        Archive::<Zip<_>>::read(INVALID).is_err_and(|e| e == ZipError::InvalidSignature("Eocdr64"))
-    );
-
-    assert!(Archive::<Zip<_>>::read(OVERLAP).is_err_and(|e| e
-        == ZipError::Overlapping(
-            "Central directory records",
-            "End of central directory record"
-        )));
-
-    assert!(Archive::<Zip<_>>::read(OVERLAP64).is_err_and(|e| e
-        == ZipError::Overlapping(
-            "Central directory records",
-            "Zip64 end of central directory record"
-        )));
+    assert!(Archive::<Zip<_>>::read(Cursor::new(&[]))
+        .is_err_and(|e| e == ZipError::StructNotFound("Eocdr")));
+
+    assert!(Archive::<Zip<_>>::read(Cursor::new(&[
+        // No Eocdr64
+        0x50, 0x4b, 0x06, 0x07, 0, 0, 0, 0, //
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr64Locator
+        //
+        0x50, 0x4b, 0x05, 0x06, 0, 0, 0, 0, 0, //
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr
+    ]))
+    .is_err_and(|e| e == ZipError::InvalidSignature("Eocdr64")));
+
+    assert!(Archive::<Zip<_>>::read(Cursor::new(&[
+        0x50, 0x4b, 0x06, 0x06, // Eocdr64
+        //
+        0x50, 0x4b, 0x06, 0x07, 0, 0, 0, 0, //
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr64Locator
+        //
+        0x50, 0x4b, 0x05, 0x06, 0, 0, 0, 0, 0, //
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr
+    ]))
+    .is_err_and(|e| e == ZipError::Overlapping("Eocdr64", "Eocdr64Locator")));
+
+    assert!(Archive::<Zip<_>>::read(Cursor::new(&[
+        // No records
+        0x50, 0x4b, 0x05, 0x06, 0, 0, 0, 0, 0, //
+        0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr
+    ]))
+    .is_err_and(|e| e == ZipError::Overlapping("Cdr", "Eocdr")));
+
+    assert!(Archive::<Zip<_>>::read(Cursor::new(&[
+        0x50, 0x4b, 0x06, 0x06, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, //
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, //
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr64
+        //
+        0x50, 0x4b, 0x06, 0x07, 0, 0, 0, 0, //
+        0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr64Locator
+        //
+        0x50, 0x4b, 0x05, 0x06, 0, 0, 0, 0, 0, //
+        0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, // Eocdr
+    ]))
+    .is_err_and(|e| e == ZipError::Overlapping("Cdr", "Eocdr64")));
 }