fix(xfl): validate archive record boundaries

crates/xfl/src/lib.rs

@@ -203,12 +203,12 @@ fn clean_path(name: &str) -> Result<String, ZipError> {
         return Err(ZipError::Unsafe(shown()));
     }
     let mut parts = Vec::new();
-    for (i, part) in unix.split('/').enumerate() {
+    for part in unix.split('/') {
         match part {
             "" | "." => {}
             ".." => return Err(ZipError::Unsafe(shown())),
             // `C:`, `C:x`: a drive.
-            p if i == 0 && p.contains(':') => return Err(ZipError::Unsafe(shown())),
+            p if parts.is_empty() && p.contains(':') => return Err(ZipError::Unsafe(shown())),
             p => parts.push(p),
         }
     }
@@ -234,7 +234,13 @@ fn method_name(method: u16) -> &'static str {
 fn find_end(data: &[u8]) -> Result<(u64, u64, u64, usize), ZipError> {
     // The end record is in the last 22 + 65535 bytes (it may carry a comment).
     let lo = data.len().saturating_sub(22 + 65_535);
-    let eocd = (lo..data.len().saturating_sub(21)).rev().find(|&i| u32_at(data, i) == Some(END)).ok_or(ZipError::NotZip)?;
+    let eocd = (lo..data.len().saturating_sub(21))
+        .rev()
+        .find(|&i| {
+            u32_at(data, i) == Some(END)
+                && u16_at(data, i + 20).is_some_and(|comment| i.checked_add(22 + usize::from(comment)).is_some_and(|end| end <= data.len()))
+        })
+        .ok_or(ZipError::NotZip)?;
     let field16 = |o: usize| u16_at(data, eocd + o).ok_or(ZipError::Damaged("the end record is cut short"));
     let field32 = |o: usize| u32_at(data, eocd + o).ok_or(ZipError::Damaged("the end record is cut short"));
     let (disk, dir_disk, on_disk, count) = (field16(4)?, field16(6)?, field16(8)?, field16(10)?);
@@ -346,6 +352,9 @@ impl<'a> Archive<'a> {
                 return Err(ZipError::Duplicate(shown()));
             }
         }
+        if i != dir.len() {
+            return Err(ZipError::Damaged("the central directory has trailing data"));
+        }
         // No two entries may share bytes (the overlapping-entries zip bomb).
         spans.sort_unstable_by_key(|s| s.0);
         if let Some(w) = spans.windows(2).find(|w| w[0].1 > w[1].0) {

crates/xfl/src/tests.rs

@@ -219,6 +219,37 @@ fn duplicate_names_are_refused() {
     assert_eq!(Archive::open(&z).unwrap().counts(), (1, 2));
 }
 
+#[test]
+fn drive_prefix_after_normalized_dot_segments_is_refused() {
+    for name in ["./C:/LIBRARY/a.xml", "./C:\\LIBRARY\\a.xml"] {
+        assert!(matches!(Archive::open(&zip(&[(name, b"x", false)])).err(), Some(ZipError::Unsafe(_))));
+    }
+    assert!(Archive::open(&zip(&[("./LIBRARY/a.xml", b"x", false)])).is_ok());
+}
+
+#[test]
+fn invalid_eocd_comment_candidate_does_not_mask_archive_end() {
+    let mut z = zip(&[("a.xml", b"x", false)]);
+    let end = z.len() - 22;
+    z[end + 20..end + 22].copy_from_slice(&22u16.to_le_bytes());
+    let fake = z.len();
+    z.extend_from_slice(&END.to_le_bytes());
+    z.extend_from_slice(&[0; 16]);
+    z.extend_from_slice(&u16::MAX.to_le_bytes());
+    assert!(Archive::open(&z).is_ok());
+    assert_eq!(fake + 22, z.len());
+}
+
+#[test]
+fn central_directory_must_be_consumed_exactly() {
+    let mut z = zip(&[("a.xml", b"x", false)]);
+    let end = z.len() - 22;
+    let size = u32::from_le_bytes(z[end + 12..end + 16].try_into().unwrap());
+    z.insert(end, 0);
+    z[end + 12 + 1..end + 16 + 1].copy_from_slice(&(size + 1).to_le_bytes());
+    assert_eq!(Archive::open(&z).err(), Some(ZipError::Damaged("the central directory has trailing data")));
+}
+
 #[test]
 fn deeply_nested_folders_are_refused() {
     let ok = format!("{}a.xml", "f/".repeat(MAX_DEPTH));