fix: Fails to extract file which might or might not be malformed (#376)

README.md

@@ -39,7 +39,8 @@ The features available are:
 * `deflate-flate2`: Combine this with any `flate2` feature flag that enables a back-end, to support deflate compression
   at quality 1..=9.
 * `deflate-zopfli`: Enables deflating files with the `zopfli` library (used when compression quality is 10..=264). This
-  is the most effective `deflate` implementation available, but also among the slowest.
+  is the most effective `deflate` implementation available, but also among the slowest. If `flate2` isn't also enabled,
+  only compression will be supported and not decompression.
 * `deflate64`: Enables the deflate64 compression algorithm. Only decompression is supported.
 * `lzma`: Enables the LZMA compression algorithm. Only decompression is supported.
 * `bzip2`: Enables the BZip2 compression algorithm.

src/path.rs

@@ -10,7 +10,9 @@ pub(crate) fn simplified_components(input: &Path) -> Option<Vec<&OsStr>> {
     let mut out = Vec::new();
     for component in input.components() {
         match component {
-            Component::Prefix(_) | Component::RootDir => return None,
+            // Skip prefix and root directory components instead of rejecting the entire path
+            // This allows extraction of ZIP files with absolute paths, similar to other ZIP tools
+            Component::Prefix(_) | Component::RootDir => (),
             Component::ParentDir => {
                 out.pop()?;
             }
@@ -20,3 +22,90 @@ pub(crate) fn simplified_components(input: &Path) -> Option<Vec<&OsStr>> {
     }
     Some(out)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::path::Path;
+
+    #[test]
+    fn test_simplified_components_relative_path() {
+        let path = Path::new("foo/bar/baz.txt");
+        let components = simplified_components(path).unwrap();
+        assert_eq!(components.len(), 3);
+        assert_eq!(components[0], "foo");
+        assert_eq!(components[1], "bar");
+        assert_eq!(components[2], "baz.txt");
+    }
+
+    #[test]
+    fn test_simplified_components_absolute_unix_path() {
+        let path = Path::new("/foo/bar/baz.txt");
+        let components = simplified_components(path).unwrap();
+        assert_eq!(components.len(), 3);
+        assert_eq!(components[0], "foo");
+        assert_eq!(components[1], "bar");
+        assert_eq!(components[2], "baz.txt");
+    }
+
+    #[test]
+    fn test_simplified_components_with_parent_dirs() {
+        let path = Path::new("foo/../bar/baz.txt");
+        let components = simplified_components(path).unwrap();
+        assert_eq!(components.len(), 2);
+        assert_eq!(components[0], "bar");
+        assert_eq!(components[1], "baz.txt");
+    }
+
+    #[test]
+    fn test_simplified_components_too_many_parent_dirs() {
+        let path = Path::new("foo/../../bar");
+        let result = simplified_components(path);
+        assert!(result.is_none()); // Should still fail for directory traversal attacks
+    }
+
+    #[test]
+    fn test_simplified_components_with_current_dir() {
+        let path = Path::new("foo/./bar/baz.txt");
+        let components = simplified_components(path).unwrap();
+        assert_eq!(components.len(), 3);
+        assert_eq!(components[0], "foo");
+        assert_eq!(components[1], "bar");
+        assert_eq!(components[2], "baz.txt");
+    }
+
+    #[test]
+    fn test_simplified_components_empty_path() {
+        let path = Path::new("");
+        let components = simplified_components(path).unwrap();
+        assert_eq!(components.len(), 0);
+    }
+
+    #[test]
+    fn test_simplified_components_root_only() {
+        let path = Path::new("/");
+        let components = simplified_components(path).unwrap();
+        assert_eq!(components.len(), 0);
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn test_simplified_components_windows_absolute_path() {
+        let path = Path::new(r"C:\foo\bar\baz.txt");
+        let components = simplified_components(path).unwrap();
+        assert_eq!(components.len(), 3);
+        assert_eq!(components[0], "foo");
+        assert_eq!(components[1], "bar");
+        assert_eq!(components[2], "baz.txt");
+    }
+
+    #[cfg(windows)]
+    #[test]
+    fn test_simplified_components_windows_unc_path() {
+        let path = Path::new(r"\\server\share\foo\bar.txt");
+        let components = simplified_components(path).unwrap();
+        assert_eq!(components.len(), 2);
+        assert_eq!(components[0], "foo");
+        assert_eq!(components[1], "bar.txt");
+    }
+}

src/types.rs

@@ -603,7 +603,12 @@ impl ZipFileData {
         let mut depth = 0usize;
         for component in path.components() {
             match component {
-                Component::Prefix(_) | Component::RootDir => return None,
+                Component::Prefix(_) | Component::RootDir => {
+                    if depth > 0 {
+                        return None;
+                    }
+                    // else absolute path becomes relative to destination instead
+                }
                 Component::ParentDir => depth = depth.checked_sub(1)?,
                 Component::Normal(_) => depth += 1,
                 Component::CurDir => (),

test_fix.rs

@@ -0,0 +1,72 @@
+use std::io::Write;
+use zip::write::SimpleFileOptions;
+use zip::{ZipArchive, ZipWriter};
+
+fn main() {
+    println!("Testing the fix for absolute paths in ZIP files...");
+
+    // Create a ZIP file with absolute paths
+    let buf = Vec::new();
+    let mut writer = ZipWriter::new(std::io::Cursor::new(buf));
+    let options = SimpleFileOptions::default();
+
+    // Create entries with absolute paths
+    writer.add_directory("/_/", options).unwrap();
+    writer.start_file("/_/test.txt", options).unwrap();
+    writer.write_all(b"Hello, World!").unwrap();
+    writer.start_file("/_/subdir/nested.txt", options).unwrap();
+    writer.write_all(b"Nested file content").unwrap();
+
+    let zip_data = writer.finish().unwrap().into_inner();
+
+    // Try to read the ZIP file
+    let mut archive = ZipArchive::new(std::io::Cursor::new(zip_data)).unwrap();
+
+    println!("ZIP file created with {} entries", archive.len());
+
+    // Test individual file access
+    for i in 0..archive.len() {
+        let file = archive.by_index(i).unwrap();
+        println!("File {}: {}", i, file.name());
+
+        let enclosed_name = file.enclosed_name();
+        match enclosed_name {
+            Some(path) => {
+                println!("  Enclosed name: {:?}", path);
+                println!("  Is absolute: {}", path.is_absolute());
+            }
+            None => {
+                println!("  Enclosed name: None (invalid path)");
+            }
+        }
+    }
+
+    // Try to extract the ZIP file
+    let temp_dir = tempfile::TempDir::new().unwrap();
+    println!("Extracting to: {:?}", temp_dir.path());
+
+    match archive.extract(temp_dir.path()) {
+        Ok(_) => {
+            println!("✅ Extraction successful!");
+
+            // Check if files were extracted correctly
+            let extracted_files = std::fs::read_dir(temp_dir.path()).unwrap();
+            for entry in extracted_files {
+                let entry = entry.unwrap();
+                println!("  Extracted: {:?}", entry.path());
+            }
+
+            // Check specific files
+            let test_file = temp_dir.path().join("_/test.txt");
+            if test_file.exists() {
+                let content = std::fs::read_to_string(&test_file).unwrap();
+                println!("  Content of _/test.txt: {}", content);
+            }
+        }
+        Err(e) => {
+            println!("❌ Extraction failed: {}", e);
+        }
+    }
+
+    println!("Test completed!");
+}

tests/invalid_path.rs

@@ -0,0 +1,180 @@
+#[cfg(all(
+    test,
+    not(all(feature = "deflate-zopfli", not(feature = "deflate-flate2")))
+))]
+pub mod tests {
+    use std::io::Write;
+    use zip::write::SimpleFileOptions;
+    use zip::{ZipArchive, ZipWriter};
+
+    /// Create a ZIP file with entries that have absolute paths (starting with /)
+    /// This simulates the problematic ZIP file mentioned in the bug report
+    fn create_zip_with_absolute_paths() -> Vec<u8> {
+        let buf = Vec::new();
+        let mut writer = ZipWriter::new(std::io::Cursor::new(buf));
+        let options = SimpleFileOptions::default();
+
+        // Create entries with absolute paths - this should cause "Invalid file path" error
+        writer.add_directory("/_/", options).unwrap();
+        writer.start_file("/_/file1.txt", options).unwrap();
+        writer.write_all(b"File 1 content").unwrap();
+        writer.start_file("/_/subdir/file2.txt", options).unwrap();
+        writer.write_all(b"File 2 content").unwrap();
+
+        writer.finish().unwrap().into_inner()
+    }
+
+    /// Create a ZIP file with entries that have Windows-style absolute paths
+    fn create_zip_with_windows_absolute_paths() -> Vec<u8> {
+        let buf = Vec::new();
+        let mut writer = ZipWriter::new(std::io::Cursor::new(buf));
+        let options = SimpleFileOptions::default();
+
+        // Create entries with Windows absolute paths
+        writer.add_directory("C:\\temp\\", options).unwrap();
+        writer.start_file("C:\\temp\\file1.txt", options).unwrap();
+        writer.write_all(b"File 1 content").unwrap();
+
+        writer.finish().unwrap().into_inner()
+    }
+
+    /// Create a ZIP file that more closely simulates the soldeer registry issue
+    /// with an underscore directory at the root with absolute path
+    fn create_zip_like_soldeer_issue() -> Vec<u8> {
+        let buf = Vec::new();
+        let mut writer = ZipWriter::new(std::io::Cursor::new(buf));
+        let options = SimpleFileOptions::default();
+
+        // Simulate the soldeer registry structure with absolute paths
+        writer.add_directory("/_/", options).unwrap();
+        writer.add_directory("/_/forge-std/", options).unwrap();
+        writer
+            .start_file("/_/forge-std/src/Test.sol", options)
+            .unwrap();
+        writer
+            .write_all(b"// SPDX-License-Identifier: MIT\npragma solidity ^0.8.0;\n")
+            .unwrap();
+        writer
+            .start_file("/_/forge-std/lib/ds-test/src/test.sol", options)
+            .unwrap();
+        writer.write_all(b"// Test contract\n").unwrap();
+
+        writer.finish().unwrap().into_inner()
+    }
+
+    #[test]
+    fn test_extract_zip_with_absolute_paths() {
+        let zip_data = create_zip_with_absolute_paths();
+        let mut archive = ZipArchive::new(std::io::Cursor::new(zip_data)).unwrap();
+
+        // After fix: should extract successfully, stripping the leading /
+        let temp_dir = tempfile::TempDir::new().unwrap();
+        archive.extract(temp_dir.path()).unwrap();
+
+        // Files should be extracted with the absolute path prefix stripped
+        assert!(temp_dir.path().join("_").exists());
+        assert!(temp_dir.path().join("_/file1.txt").exists());
+        assert!(temp_dir.path().join("_/subdir/file2.txt").exists());
+
+        // Verify file contents
+        let content = std::fs::read_to_string(temp_dir.path().join("_/file1.txt")).unwrap();
+        assert_eq!(content, "File 1 content");
+    }
+
+    #[test]
+    fn test_extract_zip_with_windows_absolute_paths() {
+        let zip_data = create_zip_with_windows_absolute_paths();
+        let mut archive = ZipArchive::new(std::io::Cursor::new(zip_data)).unwrap();
+
+        // After fix: should extract successfully, stripping the C:\ prefix
+        let temp_dir = tempfile::TempDir::new().unwrap();
+        archive.extract(temp_dir.path()).unwrap();
+
+        // Files should be extracted with the Windows absolute path prefix stripped
+        assert!(temp_dir.path().join("temp").exists());
+        assert!(temp_dir.path().join("temp/file1.txt").exists());
+
+        // Verify file contents
+        let content = std::fs::read_to_string(temp_dir.path().join("temp/file1.txt")).unwrap();
+        assert_eq!(content, "File 1 content");
+    }
+
+    #[test]
+    fn test_extract_soldeer_like_zip() {
+        let zip_data = create_zip_like_soldeer_issue();
+        let mut archive = ZipArchive::new(std::io::Cursor::new(zip_data)).unwrap();
+
+        // This should now work without "Invalid file path" error
+        let temp_dir = tempfile::TempDir::new().unwrap();
+        archive.extract(temp_dir.path()).unwrap();
+
+        // Verify the structure is extracted correctly with absolute prefix stripped
+        assert!(temp_dir.path().join("_").exists());
+        assert!(temp_dir.path().join("_/forge-std").exists());
+        assert!(temp_dir.path().join("_/forge-std/src/Test.sol").exists());
+        assert!(temp_dir
+            .path()
+            .join("_/forge-std/lib/ds-test/src/test.sol")
+            .exists());
+
+        // Verify file contents
+        let content =
+            std::fs::read_to_string(temp_dir.path().join("_/forge-std/src/Test.sol")).unwrap();
+        assert!(content.contains("SPDX-License-Identifier"));
+    }
+
+    #[test]
+    fn test_individual_file_access_with_absolute_paths() {
+        let zip_data = create_zip_with_absolute_paths();
+        let mut archive = ZipArchive::new(std::io::Cursor::new(zip_data)).unwrap();
+
+        // Test accessing individual files
+        for i in 0..archive.len() {
+            let file = archive.by_index(i).unwrap();
+            println!("File name: {}", file.name());
+
+            // After our fix, enclosed_name should return a safe relative path
+            let enclosed_name = file.enclosed_name();
+            println!("Enclosed name: {:?}", enclosed_name);
+
+            // Should now return Some with the absolute prefix stripped
+            assert!(enclosed_name.is_some());
+            let path = enclosed_name.unwrap();
+
+            // Verify the path doesn't start with / or contain absolute components
+            assert!(!path.is_absolute());
+            assert!(!path.to_string_lossy().starts_with('/'));
+        }
+    }
+
+    #[test]
+    fn test_security_still_prevents_directory_traversal() {
+        let buf = Vec::new();
+        let mut writer = ZipWriter::new(std::io::Cursor::new(buf));
+        let options = SimpleFileOptions::default();
+
+        // Create a ZIP with directory traversal attempts
+        writer.start_file("../../../etc/passwd", options).unwrap();
+        writer.write_all(b"malicious content").unwrap();
+        writer
+            .start_file("foo/../../../etc/shadow", options)
+            .unwrap();
+        writer.write_all(b"more malicious content").unwrap();
+
+        let zip_data = writer.finish().unwrap().into_inner();
+        let mut archive = ZipArchive::new(std::io::Cursor::new(zip_data)).unwrap();
+
+        // These should still fail due to directory traversal protection
+        for i in 0..archive.len() {
+            let file = archive.by_index(i).unwrap();
+            let enclosed_name = file.enclosed_name();
+
+            // Directory traversal attempts should still return None
+            assert!(
+                enclosed_name.is_none(),
+                "Directory traversal should still be blocked for: {}",
+                file.name()
+            );
+        }
+    }
+}