Skip to content

Commit f326458

Browse files
1 parent 8d3408f commit f326458

File tree

3 files changed

+71
-3
lines changed

3 files changed

+71
-3
lines changed

crates/uv-extract/src/stream.rs

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
1-
use std::path::Path;
1+
use std::path::{Component, Path, PathBuf};
22
use std::pin::Pin;
33

4-
use crate::Error;
54
use futures::StreamExt;
65
use rustc_hash::FxHashSet;
76
use tokio_util::compat::{FuturesAsyncReadCompatExt, TokioAsyncReadCompatExt};
87
use tracing::warn;
8+
99
use uv_distribution_filename::SourceDistExtension;
1010

11+
use crate::Error;
12+
1113
const DEFAULT_BUF_SIZE: usize = 128 * 1024;
1214

1315
/// Unpack a `.zip` archive into the target directory, without requiring `Seek`.
@@ -19,6 +21,26 @@ pub async fn unzip<R: tokio::io::AsyncRead + Unpin>(
1921
reader: R,
2022
target: impl AsRef<Path>,
2123
) -> Result<(), Error> {
24+
/// Ensure the file path is safe to use as a [`Path`].
25+
///
26+
/// See: <https://docs.rs/zip/latest/zip/read/struct.ZipFile.html#method.enclosed_name>
27+
pub(crate) fn enclosed_name(file_name: &str) -> Option<PathBuf> {
28+
if file_name.contains('\0') {
29+
return None;
30+
}
31+
let path = PathBuf::from(file_name);
32+
let mut depth = 0usize;
33+
for component in path.components() {
34+
match component {
35+
Component::Prefix(_) | Component::RootDir => return None,
36+
Component::ParentDir => depth = depth.checked_sub(1)?,
37+
Component::Normal(_) => depth += 1,
38+
Component::CurDir => (),
39+
}
40+
}
41+
Some(path)
42+
}
43+
2244
let target = target.as_ref();
2345
let mut reader = futures::io::BufReader::with_capacity(DEFAULT_BUF_SIZE, reader.compat());
2446
let mut zip = async_zip::base::read::stream::ZipFileReader::new(&mut reader);
@@ -28,6 +50,16 @@ pub async fn unzip<R: tokio::io::AsyncRead + Unpin>(
2850
while let Some(mut entry) = zip.next_with_entry().await? {
2951
// Construct the (expected) path to the file on-disk.
3052
let path = entry.reader().entry().filename().as_str()?;
53+
54+
// Sanitize the file name to prevent directory traversal attacks.
55+
let Some(path) = enclosed_name(path) else {
56+
warn!("Skipping unsafe file name: {path}");
57+
58+
// Close current file prior to proceeding, as per:
59+
// https://docs.rs/async_zip/0.0.16/async_zip/base/read/stream/
60+
zip = entry.skip().await?;
61+
continue;
62+
};
3163
let path = target.join(path);
3264
let is_dir = entry.reader().entry().dir()?;
3365

@@ -55,7 +87,7 @@ pub async fn unzip<R: tokio::io::AsyncRead + Unpin>(
5587
tokio::io::copy(&mut reader, &mut writer).await?;
5688
}
5789

58-
// Close current file to get access to the next one. See docs:
90+
// Close current file prior to proceeding, as per:
5991
// https://docs.rs/async_zip/0.0.16/async_zip/base/read/stream/
6092
zip = entry.skip().await?;
6193
}
@@ -84,6 +116,9 @@ pub async fn unzip<R: tokio::io::AsyncRead + Unpin>(
84116
if has_any_executable_bit != 0 {
85117
// Construct the (expected) path to the file on-disk.
86118
let path = entry.filename().as_str()?;
119+
let Some(path) = enclosed_name(path) else {
120+
continue;
121+
};
87122
let path = target.join(path);
88123

89124
let permissions = fs_err::tokio::metadata(&path).await?.permissions();

crates/uv-extract/src/sync.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::sync::Mutex;
33

44
use rayon::prelude::*;
55
use rustc_hash::FxHashSet;
6+
use tracing::warn;
67
use zip::ZipArchive;
78

89
use crate::vendor::{CloneableSeekableReader, HasLength};
@@ -25,6 +26,7 @@ pub fn unzip<R: Send + std::io::Read + std::io::Seek + HasLength>(
2526

2627
// Determine the path of the file within the wheel.
2728
let Some(enclosed_name) = file.enclosed_name() else {
29+
warn!("Skipping unsafe file name: {}", file.name());
2830
return Ok(());
2931
};
3032

crates/uv/tests/it/pip_sync.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5607,3 +5607,34 @@ fn sync_seed() -> Result<()> {
56075607

56085608
Ok(())
56095609
}
5610+
5611+
/// Sanitize zip files during extraction.
5612+
#[test]
5613+
fn sanitize() -> Result<()> {
5614+
let context = TestContext::new("3.12");
5615+
5616+
// Install a zip file that includes a path that extends outside the parent.
5617+
let requirements_txt = context.temp_dir.child("requirements.txt");
5618+
requirements_txt.write_str("payload-package @ https://github.com/astral-sh/sanitize-wheel-test/raw/bc59283d5b4b136a191792e32baa51b477fdf65e/payload_package-0.1.0-py3-none-any.whl")?;
5619+
5620+
uv_snapshot!(context.pip_sync()
5621+
.arg("requirements.txt"), @r###"
5622+
success: true
5623+
exit_code: 0
5624+
----- stdout -----
5625+
5626+
----- stderr -----
5627+
Resolved 1 package in [TIME]
5628+
Prepared 1 package in [TIME]
5629+
Installed 1 package in [TIME]
5630+
+ payload-package==0.1.0 (from https://github.com/astral-sh/sanitize-wheel-test/raw/bc59283d5b4b136a191792e32baa51b477fdf65e/payload_package-0.1.0-py3-none-any.whl)
5631+
"###
5632+
);
5633+
5634+
// There should be no `payload` file in the root.
5635+
if let Some(parent) = context.temp_dir.parent() {
5636+
assert!(!parent.join("payload").exists());
5637+
}
5638+
5639+
Ok(())
5640+
}

0 commit comments

Comments
 (0)