From 18b8948097126df9690934c2a3d580c3c2a3e924 Mon Sep 17 00:00:00 2001 From: "amazon-q-developer[bot]" <208079219+amazon-q-developer[bot]@users.noreply.github.com> Date: Fri, 12 Sep 2025 01:42:00 +0000 Subject: [PATCH 1/3] AI Generated --- src/path.rs | 91 +++++++++++++++++++++++- test_fix.rs | 72 +++++++++++++++++++ tests/invalid_path.rs | 158 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 320 insertions(+), 1 deletion(-) create mode 100644 test_fix.rs create mode 100644 tests/invalid_path.rs diff --git a/src/path.rs b/src/path.rs index 69ef75da1..d6ea2e059 100644 --- a/src/path.rs +++ b/src/path.rs @@ -10,7 +10,9 @@ pub(crate) fn simplified_components(input: &Path) -> Option> { 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> { } 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"); + } +} diff --git a/test_fix.rs b/test_fix.rs new file mode 100644 index 000000000..bb4252d80 --- /dev/null +++ b/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!"); +} \ No newline at end of file diff --git a/tests/invalid_path.rs b/tests/invalid_path.rs new file mode 100644 index 000000000..dfd0a8caf --- /dev/null +++ b/tests/invalid_path.rs @@ -0,0 +1,158 @@ +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 { + 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 { + 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 { + 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()); + } +} \ No newline at end of file From e124d048a2b7ede9f19c3e2899b995679e73bbde Mon Sep 17 00:00:00 2001 From: hennickc Date: Tue, 16 Sep 2025 12:30:45 -0700 Subject: [PATCH 2/3] test: Fix: new tests don't work with zopfli and no flate2 --- README.md | 3 +- tests/invalid_path.rs | 316 ++++++++++++++++++++++-------------------- 2 files changed, 171 insertions(+), 148 deletions(-) diff --git a/README.md b/README.md index 9352e9b45..d5a8fd480 100644 --- a/README.md +++ b/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. diff --git a/tests/invalid_path.rs b/tests/invalid_path.rs index dfd0a8caf..588331b05 100644 --- a/tests/invalid_path.rs +++ b/tests/invalid_path.rs @@ -1,158 +1,180 @@ -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 { - 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() -} +#[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 { + 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 { - let buf = Vec::new(); - let mut writer = ZipWriter::new(std::io::Cursor::new(buf)); - let options = SimpleFileOptions::default(); + /// Create a ZIP file with entries that have Windows-style absolute paths + fn create_zip_with_windows_absolute_paths() -> Vec { + 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(); + // 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() -} + 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 { - 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() -} + /// 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 { + 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_absolute_paths() { + let zip_data = create_zip_with_absolute_paths(); + let mut archive = ZipArchive::new(std::io::Cursor::new(zip_data)).unwrap(); -#[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"); -} + // After fix: should extract successfully, stripping the leading / + let temp_dir = tempfile::TempDir::new().unwrap(); + archive.extract(temp_dir.path()).unwrap(); -#[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")); -} + // 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()); -#[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('/')); + // 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_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()); + #[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('/')); + } } -} \ No newline at end of file + + #[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() + ); + } + } +} From 62353cafd65b25705e452ce0116113600f43f86f Mon Sep 17 00:00:00 2001 From: hennickc Date: Tue, 16 Sep 2025 12:34:52 -0700 Subject: [PATCH 3/3] fix: enclosed_name interprets absolute paths as relative to destination roots --- src/types.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/types.rs b/src/types.rs index a076d1f4f..92e229803 100644 --- a/src/types.rs +++ b/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 => (),