Keep ZIP64 values within their extra field (from #701)

crates/formats/src/zip.rs

@@ -149,34 +149,38 @@ pub fn parse(data: &[u8], max_entries: usize) -> Result<ZipArchive, ZipError> {
         let unix_mode = if host == 3 { r.u32_le(pos + 38).map(|a| a >> 16) } else { None };
         let mut local = u64::from(r.u32_le(pos + 42).ok_or(ZipError::Truncated("directory entry"))?);
         let name_bytes = r.slice(pos + 46, name_len).ok_or(ZipError::Truncated("entry name"))?;
-        let extra = r.slice(pos + 46 + name_len, extra_len).unwrap_or(&[]);
-        // ZIP64 extended information extra field.
+        let extra = r.slice(pos + 46 + name_len, extra_len).ok_or(ZipError::Truncated("entry extra fields"))?;
+        // Each extra field owns its payload: ZIP64 values cannot borrow a following field's bytes.
+        let needs_zip64 = usize_ == 0xffff_ffff || csize == 0xffff_ffff || local == 0xffff_ffff;
+        let mut found_zip64 = false;
         let mut x = 0usize;
         let er = Reader::new(extra);
-        while let (Some(id), Some(len)) = (er.u16_le(x), er.u16_le(x + 2)) {
+        while x < extra.len() {
+            let id = er.u16_le(x).ok_or(ZipError::Truncated("extra field header"))?;
+            let len = usize::from(er.u16_le(x + 2).ok_or(ZipError::Truncated("extra field header"))?);
             let body = x + 4;
+            let field = er.slice(body, len).ok_or(ZipError::Truncated("extra field payload"))?;
             if id == 0x0001 {
-                let mut f = body;
-                if usize_ == 0xffff_ffff
-                    && let Some(v) = er.u64_le(f)
-                {
-                    usize_ = v;
+                let values = Reader::new(field);
+                let mut f = 0;
+                if usize_ == 0xffff_ffff {
+                    usize_ = values.u64_le(f).ok_or(ZipError::Truncated("zip64 uncompressed size"))?;
                     f += 8;
                 }
-                if csize == 0xffff_ffff
-                    && let Some(v) = er.u64_le(f)
-                {
-                    csize = v;
+                if csize == 0xffff_ffff {
+                    csize = values.u64_le(f).ok_or(ZipError::Truncated("zip64 compressed size"))?;
                     f += 8;
                 }
-                if local == 0xffff_ffff
-                    && let Some(v) = er.u64_le(f)
-                {
-                    local = v;
+                if local == 0xffff_ffff {
+                    local = values.u64_le(f).ok_or(ZipError::Truncated("zip64 local header offset"))?;
                 }
+                found_zip64 = true;
                 break;
             }
-            x = body + usize::from(len);
+            x = body + len;
+        }
+        if needs_zip64 && !found_zip64 {
+            return Err(ZipError::Corrupt("missing ZIP64 extended information".into()));
         }
         let name = decode_name(name_bytes, flags).replace('\\', "/");
         entries.push(ZipEntry {

crates/formats/tests/zip64_extra_boundaries.rs

@@ -0,0 +1,50 @@
+use craft_anything_formats::zip;
+
+fn archive(extra: &[u8], sentinel: usize) -> Vec<u8> {
+    let mut data = craft_anything_testkit::electron::zip_stored(&[("a", b"x")]);
+    let central = data.windows(4).position(|w| w == b"PK\x01\x02").unwrap();
+    let end = data.len() - 22;
+    data[central + sentinel..central + sentinel + 4].copy_from_slice(&u32::MAX.to_le_bytes());
+    data[central + 30..central + 32].copy_from_slice(&(extra.len() as u16).to_le_bytes());
+    data.splice(end..end, extra.iter().copied());
+    let end = data.len() - 22;
+    data[end + 12..end + 16].copy_from_slice(&((47 + extra.len()) as u32).to_le_bytes());
+    data
+}
+
+#[test]
+fn a_short_zip64_field_cannot_borrow_values_from_its_neighbor() {
+    for field in [20, 24, 42] {
+        let mut extra = vec![1, 0, 0, 0, 2, 0, 8, 0];
+        extra.extend_from_slice(&1u64.to_le_bytes());
+        assert!(zip::parse(&archive(&extra, field), 10).is_err(), "field {field}");
+    }
+}
+
+#[test]
+fn zip64_sentinels_require_complete_values() {
+    for field in [20, 24, 42] {
+        assert!(zip::parse(&archive(&[], field), 10).is_err());
+        assert!(zip::parse(&archive(&[1, 0, 7, 0, 1, 0, 0, 0, 0, 0, 0], field), 10).is_err());
+    }
+}
+
+#[test]
+fn a_complete_zip64_field_preserves_the_value() {
+    for field in [20, 24, 42] {
+        let value = if field == 42 { 0u64 } else { 1u64 };
+        let mut extra = vec![1, 0, 8, 0];
+        extra.extend_from_slice(&value.to_le_bytes());
+        let data = archive(&extra, field);
+        let parsed = zip::parse(&data, 10).unwrap();
+        assert_eq!(zip::read_entry(&data, &parsed.entries[0], 1).unwrap(), b"x");
+    }
+}
+
+#[test]
+fn a_legitimate_64_bit_value_equal_to_the_32_bit_sentinel_is_preserved() {
+    let mut extra = vec![1, 0, 8, 0];
+    extra.extend_from_slice(&u64::from(u32::MAX).to_le_bytes());
+    let archive = zip::parse(&archive(&extra, 24), 10).unwrap();
+    assert_eq!(archive.entries[0].uncompressed_size, u64::from(u32::MAX));
+}