fix(cli): allows extracting files without separate directory entries

Some zip files do not list directories separately from files. For example, a zip might contain `dir/file.txt` without a corresponding entry for just `dir/`. This should be okay, so we just create the leading paths for the files we extract as necessary.

Also adds tests for the expected and dangerous path cases of `extract_zip`.
This commit is contained in:
Brian Donovan 2023-08-04 21:28:09 -07:00
parent 14e7c50478
commit 03e02591ec
No known key found for this signature in database
4 changed files with 56 additions and 2 deletions

View file

@ -97,3 +97,6 @@ plugin = ["mlua"]
[[bin]]
path = "src/main.rs"
name = "dx"
[dev-dependencies]
tempfile = "3.3"

View file

@ -329,6 +329,9 @@ pub fn extract_zip(file: &Path, target: &Path) -> anyhow::Result<()> {
for i in 0..zip.len() {
let mut zip_entry = zip.by_index(i)?;
// check for dangerous paths
// see https://docs.rs/zip/latest/zip/read/struct.ZipFile.html#warnings
let Some(enclosed_name) = zip_entry.enclosed_name() else {
return Err(anyhow::anyhow!(
"Refusing to unpack zip entry with potentially dangerous path: zip={} entry={:?}",
@ -336,12 +339,17 @@ pub fn extract_zip(file: &Path, target: &Path) -> anyhow::Result<()> {
zip_entry.name()
));
};
let output_path = target.join(enclosed_name);
if zip_entry.is_dir() {
// dir
std::fs::create_dir_all(output_path)?;
} else {
// file
// create parent dirs if needed
if let Some(parent) = output_path.parent() {
std::fs::create_dir_all(parent)?;
}
// extract file
let mut target_file = if !output_path.exists() {
std::fs::File::create(output_path)?
} else {
@ -353,3 +361,46 @@ pub fn extract_zip(file: &Path, target: &Path) -> anyhow::Result<()> {
Ok(())
}
#[cfg(test)]
mod test {
use super::*;
use std::path::PathBuf;
use tempfile::tempdir;
#[test]
fn test_extract_zip() -> anyhow::Result<()> {
let path = PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap())
.join("tests/fixtures/test.zip");
let temp_dir = tempdir()?;
let temp_path = temp_dir.path();
extract_zip(path.as_path(), temp_path)?;
let expected_files = vec!["file1.txt", "file2.txt", "dir/file3.txt"];
for file in expected_files {
let path = temp_path.join(file);
assert!(path.exists(), "File not found: {:?}", path);
}
Ok(())
}
#[test]
fn test_extract_zip_dangerous_path() -> anyhow::Result<()> {
let path = PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap())
.join("tests/fixtures/dangerous.zip");
let temp_dir = tempdir()?;
let temp_path = temp_dir.path();
let result = extract_zip(path.as_path(), temp_path);
let err = result.unwrap_err();
assert!(err
.to_string()
.contains("Refusing to unpack zip entry with potentially dangerous path: zip="));
assert!(err.to_string().contains("entry=\"/etc/passwd\""));
Ok(())
}
}

Binary file not shown.

BIN
packages/cli/tests/fixtures/test.zip vendored Normal file

Binary file not shown.