Skip to content

Commit ce79505

Browse files
AI Generated
1 parent 9cf28cb commit ce79505

File tree

3 files changed

+320
-1
lines changed

3 files changed

+320
-1
lines changed

src/path.rs

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ pub(crate) fn simplified_components(input: &Path) -> Option<Vec<&OsStr>> {
1010
let mut out = Vec::new();
1111
for component in input.components() {
1212
match component {
13-
Component::Prefix(_) | Component::RootDir => return None,
13+
// Skip prefix and root directory components instead of rejecting the entire path
14+
// This allows extraction of ZIP files with absolute paths, similar to other ZIP tools
15+
Component::Prefix(_) | Component::RootDir => (),
1416
Component::ParentDir => {
1517
out.pop()?;
1618
}
@@ -20,3 +22,90 @@ pub(crate) fn simplified_components(input: &Path) -> Option<Vec<&OsStr>> {
2022
}
2123
Some(out)
2224
}
25+
26+
#[cfg(test)]
27+
mod tests {
28+
use super::*;
29+
use std::path::Path;
30+
31+
#[test]
32+
fn test_simplified_components_relative_path() {
33+
let path = Path::new("foo/bar/baz.txt");
34+
let components = simplified_components(path).unwrap();
35+
assert_eq!(components.len(), 3);
36+
assert_eq!(components[0], "foo");
37+
assert_eq!(components[1], "bar");
38+
assert_eq!(components[2], "baz.txt");
39+
}
40+
41+
#[test]
42+
fn test_simplified_components_absolute_unix_path() {
43+
let path = Path::new("/foo/bar/baz.txt");
44+
let components = simplified_components(path).unwrap();
45+
assert_eq!(components.len(), 3);
46+
assert_eq!(components[0], "foo");
47+
assert_eq!(components[1], "bar");
48+
assert_eq!(components[2], "baz.txt");
49+
}
50+
51+
#[test]
52+
fn test_simplified_components_with_parent_dirs() {
53+
let path = Path::new("foo/../bar/baz.txt");
54+
let components = simplified_components(path).unwrap();
55+
assert_eq!(components.len(), 2);
56+
assert_eq!(components[0], "bar");
57+
assert_eq!(components[1], "baz.txt");
58+
}
59+
60+
#[test]
61+
fn test_simplified_components_too_many_parent_dirs() {
62+
let path = Path::new("foo/../../bar");
63+
let result = simplified_components(path);
64+
assert!(result.is_none()); // Should still fail for directory traversal attacks
65+
}
66+
67+
#[test]
68+
fn test_simplified_components_with_current_dir() {
69+
let path = Path::new("foo/./bar/baz.txt");
70+
let components = simplified_components(path).unwrap();
71+
assert_eq!(components.len(), 3);
72+
assert_eq!(components[0], "foo");
73+
assert_eq!(components[1], "bar");
74+
assert_eq!(components[2], "baz.txt");
75+
}
76+
77+
#[test]
78+
fn test_simplified_components_empty_path() {
79+
let path = Path::new("");
80+
let components = simplified_components(path).unwrap();
81+
assert_eq!(components.len(), 0);
82+
}
83+
84+
#[test]
85+
fn test_simplified_components_root_only() {
86+
let path = Path::new("/");
87+
let components = simplified_components(path).unwrap();
88+
assert_eq!(components.len(), 0);
89+
}
90+
91+
#[cfg(windows)]
92+
#[test]
93+
fn test_simplified_components_windows_absolute_path() {
94+
let path = Path::new(r"C:\foo\bar\baz.txt");
95+
let components = simplified_components(path).unwrap();
96+
assert_eq!(components.len(), 3);
97+
assert_eq!(components[0], "foo");
98+
assert_eq!(components[1], "bar");
99+
assert_eq!(components[2], "baz.txt");
100+
}
101+
102+
#[cfg(windows)]
103+
#[test]
104+
fn test_simplified_components_windows_unc_path() {
105+
let path = Path::new(r"\\server\share\foo\bar.txt");
106+
let components = simplified_components(path).unwrap();
107+
assert_eq!(components.len(), 2);
108+
assert_eq!(components[0], "foo");
109+
assert_eq!(components[1], "bar.txt");
110+
}
111+
}

test_fix.rs

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
use std::io::Write;
2+
use zip::write::SimpleFileOptions;
3+
use zip::{ZipArchive, ZipWriter};
4+
5+
fn main() {
6+
println!("Testing the fix for absolute paths in ZIP files...");
7+
8+
// Create a ZIP file with absolute paths
9+
let buf = Vec::new();
10+
let mut writer = ZipWriter::new(std::io::Cursor::new(buf));
11+
let options = SimpleFileOptions::default();
12+
13+
// Create entries with absolute paths
14+
writer.add_directory("/_/", options).unwrap();
15+
writer.start_file("/_/test.txt", options).unwrap();
16+
writer.write_all(b"Hello, World!").unwrap();
17+
writer.start_file("/_/subdir/nested.txt", options).unwrap();
18+
writer.write_all(b"Nested file content").unwrap();
19+
20+
let zip_data = writer.finish().unwrap().into_inner();
21+
22+
// Try to read the ZIP file
23+
let mut archive = ZipArchive::new(std::io::Cursor::new(zip_data)).unwrap();
24+
25+
println!("ZIP file created with {} entries", archive.len());
26+
27+
// Test individual file access
28+
for i in 0..archive.len() {
29+
let file = archive.by_index(i).unwrap();
30+
println!("File {}: {}", i, file.name());
31+
32+
let enclosed_name = file.enclosed_name();
33+
match enclosed_name {
34+
Some(path) => {
35+
println!(" Enclosed name: {:?}", path);
36+
println!(" Is absolute: {}", path.is_absolute());
37+
}
38+
None => {
39+
println!(" Enclosed name: None (invalid path)");
40+
}
41+
}
42+
}
43+
44+
// Try to extract the ZIP file
45+
let temp_dir = tempfile::TempDir::new().unwrap();
46+
println!("Extracting to: {:?}", temp_dir.path());
47+
48+
match archive.extract(temp_dir.path()) {
49+
Ok(_) => {
50+
println!("✅ Extraction successful!");
51+
52+
// Check if files were extracted correctly
53+
let extracted_files = std::fs::read_dir(temp_dir.path()).unwrap();
54+
for entry in extracted_files {
55+
let entry = entry.unwrap();
56+
println!(" Extracted: {:?}", entry.path());
57+
}
58+
59+
// Check specific files
60+
let test_file = temp_dir.path().join("_/test.txt");
61+
if test_file.exists() {
62+
let content = std::fs::read_to_string(&test_file).unwrap();
63+
println!(" Content of _/test.txt: {}", content);
64+
}
65+
}
66+
Err(e) => {
67+
println!("❌ Extraction failed: {}", e);
68+
}
69+
}
70+
71+
println!("Test completed!");
72+
}

tests/invalid_path.rs

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
use std::io::Write;
2+
use zip::write::SimpleFileOptions;
3+
use zip::{ZipArchive, ZipWriter};
4+
5+
/// Create a ZIP file with entries that have absolute paths (starting with /)
6+
/// This simulates the problematic ZIP file mentioned in the bug report
7+
fn create_zip_with_absolute_paths() -> Vec<u8> {
8+
let buf = Vec::new();
9+
let mut writer = ZipWriter::new(std::io::Cursor::new(buf));
10+
let options = SimpleFileOptions::default();
11+
12+
// Create entries with absolute paths - this should cause "Invalid file path" error
13+
writer.add_directory("/_/", options).unwrap();
14+
writer.start_file("/_/file1.txt", options).unwrap();
15+
writer.write_all(b"File 1 content").unwrap();
16+
writer.start_file("/_/subdir/file2.txt", options).unwrap();
17+
writer.write_all(b"File 2 content").unwrap();
18+
19+
writer.finish().unwrap().into_inner()
20+
}
21+
22+
/// Create a ZIP file with entries that have Windows-style absolute paths
23+
fn create_zip_with_windows_absolute_paths() -> Vec<u8> {
24+
let buf = Vec::new();
25+
let mut writer = ZipWriter::new(std::io::Cursor::new(buf));
26+
let options = SimpleFileOptions::default();
27+
28+
// Create entries with Windows absolute paths
29+
writer.add_directory("C:\\temp\\", options).unwrap();
30+
writer.start_file("C:\\temp\\file1.txt", options).unwrap();
31+
writer.write_all(b"File 1 content").unwrap();
32+
33+
writer.finish().unwrap().into_inner()
34+
}
35+
36+
/// Create a ZIP file that more closely simulates the soldeer registry issue
37+
/// with an underscore directory at the root with absolute path
38+
fn create_zip_like_soldeer_issue() -> Vec<u8> {
39+
let buf = Vec::new();
40+
let mut writer = ZipWriter::new(std::io::Cursor::new(buf));
41+
let options = SimpleFileOptions::default();
42+
43+
// Simulate the soldeer registry structure with absolute paths
44+
writer.add_directory("/_/", options).unwrap();
45+
writer.add_directory("/_/forge-std/", options).unwrap();
46+
writer.start_file("/_/forge-std/src/Test.sol", options).unwrap();
47+
writer.write_all(b"// SPDX-License-Identifier: MIT\npragma solidity ^0.8.0;\n").unwrap();
48+
writer.start_file("/_/forge-std/lib/ds-test/src/test.sol", options).unwrap();
49+
writer.write_all(b"// Test contract\n").unwrap();
50+
51+
writer.finish().unwrap().into_inner()
52+
}
53+
54+
#[test]
55+
fn test_extract_zip_with_absolute_paths() {
56+
let zip_data = create_zip_with_absolute_paths();
57+
let mut archive = ZipArchive::new(std::io::Cursor::new(zip_data)).unwrap();
58+
59+
// After fix: should extract successfully, stripping the leading /
60+
let temp_dir = tempfile::TempDir::new().unwrap();
61+
archive.extract(temp_dir.path()).unwrap();
62+
63+
// Files should be extracted with the absolute path prefix stripped
64+
assert!(temp_dir.path().join("_").exists());
65+
assert!(temp_dir.path().join("_/file1.txt").exists());
66+
assert!(temp_dir.path().join("_/subdir/file2.txt").exists());
67+
68+
// Verify file contents
69+
let content = std::fs::read_to_string(temp_dir.path().join("_/file1.txt")).unwrap();
70+
assert_eq!(content, "File 1 content");
71+
}
72+
73+
#[test]
74+
fn test_extract_zip_with_windows_absolute_paths() {
75+
let zip_data = create_zip_with_windows_absolute_paths();
76+
let mut archive = ZipArchive::new(std::io::Cursor::new(zip_data)).unwrap();
77+
78+
// After fix: should extract successfully, stripping the C:\ prefix
79+
let temp_dir = tempfile::TempDir::new().unwrap();
80+
archive.extract(temp_dir.path()).unwrap();
81+
82+
// Files should be extracted with the Windows absolute path prefix stripped
83+
assert!(temp_dir.path().join("temp").exists());
84+
assert!(temp_dir.path().join("temp/file1.txt").exists());
85+
86+
// Verify file contents
87+
let content = std::fs::read_to_string(temp_dir.path().join("temp/file1.txt")).unwrap();
88+
assert_eq!(content, "File 1 content");
89+
}
90+
91+
#[test]
92+
fn test_extract_soldeer_like_zip() {
93+
let zip_data = create_zip_like_soldeer_issue();
94+
let mut archive = ZipArchive::new(std::io::Cursor::new(zip_data)).unwrap();
95+
96+
// This should now work without "Invalid file path" error
97+
let temp_dir = tempfile::TempDir::new().unwrap();
98+
archive.extract(temp_dir.path()).unwrap();
99+
100+
// Verify the structure is extracted correctly with absolute prefix stripped
101+
assert!(temp_dir.path().join("_").exists());
102+
assert!(temp_dir.path().join("_/forge-std").exists());
103+
assert!(temp_dir.path().join("_/forge-std/src/Test.sol").exists());
104+
assert!(temp_dir.path().join("_/forge-std/lib/ds-test/src/test.sol").exists());
105+
106+
// Verify file contents
107+
let content = std::fs::read_to_string(temp_dir.path().join("_/forge-std/src/Test.sol")).unwrap();
108+
assert!(content.contains("SPDX-License-Identifier"));
109+
}
110+
111+
#[test]
112+
fn test_individual_file_access_with_absolute_paths() {
113+
let zip_data = create_zip_with_absolute_paths();
114+
let mut archive = ZipArchive::new(std::io::Cursor::new(zip_data)).unwrap();
115+
116+
// Test accessing individual files
117+
for i in 0..archive.len() {
118+
let file = archive.by_index(i).unwrap();
119+
println!("File name: {}", file.name());
120+
121+
// After our fix, enclosed_name should return a safe relative path
122+
let enclosed_name = file.enclosed_name();
123+
println!("Enclosed name: {:?}", enclosed_name);
124+
125+
// Should now return Some with the absolute prefix stripped
126+
assert!(enclosed_name.is_some());
127+
let path = enclosed_name.unwrap();
128+
129+
// Verify the path doesn't start with / or contain absolute components
130+
assert!(!path.is_absolute());
131+
assert!(!path.to_string_lossy().starts_with('/'));
132+
}
133+
}
134+
135+
#[test]
136+
fn test_security_still_prevents_directory_traversal() {
137+
let buf = Vec::new();
138+
let mut writer = ZipWriter::new(std::io::Cursor::new(buf));
139+
let options = SimpleFileOptions::default();
140+
141+
// Create a ZIP with directory traversal attempts
142+
writer.start_file("../../../etc/passwd", options).unwrap();
143+
writer.write_all(b"malicious content").unwrap();
144+
writer.start_file("foo/../../../etc/shadow", options).unwrap();
145+
writer.write_all(b"more malicious content").unwrap();
146+
147+
let zip_data = writer.finish().unwrap().into_inner();
148+
let mut archive = ZipArchive::new(std::io::Cursor::new(zip_data)).unwrap();
149+
150+
// These should still fail due to directory traversal protection
151+
for i in 0..archive.len() {
152+
let file = archive.by_index(i).unwrap();
153+
let enclosed_name = file.enclosed_name();
154+
155+
// Directory traversal attempts should still return None
156+
assert!(enclosed_name.is_none(), "Directory traversal should still be blocked for: {}", file.name());
157+
}
158+
}

0 commit comments

Comments
 (0)