Do not remove files outside the venv on uninstall (#18942)
Check that only files inside the installation scheme can be removed when uv uninstalls a package. This fixes a bug where uv would try to remove arbitrary files due to a malformed or malicious RECORD file in a wheel. For venvs, the installation prefix is the entire venv, as `.data/data` allows wheels to write to the entire venv, so all files in the venv can also be removed. This is both a correctness fix (uv should never remove files outside its domain) and a low severity security fix, where a malicious wheel could remove a user's files during uninstallation, such as a `uv sync` that upgrades the package version. Note that this requires an attacker having control over the wheel, which also allows them to modify arbitrary Python code. There are no known cases of wheels actually referencing files outside the installation scheme in their RECORD.
This commit is contained in:
@@ -378,8 +378,9 @@ impl BuildContext for BuildDispatch<'_> {
|
||||
|
||||
// Remove any unnecessary packages.
|
||||
if !reinstalls.is_empty() {
|
||||
let layout = venv.interpreter().layout();
|
||||
for dist_info in &reinstalls {
|
||||
let summary = uv_installer::uninstall(dist_info)
|
||||
let summary = uv_installer::uninstall(dist_info, &layout)
|
||||
.await
|
||||
.context("Failed to uninstall build dependencies")?;
|
||||
debug!(
|
||||
|
||||
@@ -1,15 +1,22 @@
|
||||
use std::collections::BTreeSet;
|
||||
use std::collections::{BTreeSet, HashSet};
|
||||
use std::fmt::Display;
|
||||
use std::path::{Component, Path, PathBuf};
|
||||
use std::sync::{LazyLock, Mutex, OnceLock};
|
||||
|
||||
use std::sync::{LazyLock, Mutex};
|
||||
use tracing::trace;
|
||||
use uv_fs::write_atomic_sync;
|
||||
|
||||
use crate::Error;
|
||||
use uv_fs::write_atomic_sync;
|
||||
use uv_warnings::warn_user;
|
||||
|
||||
use crate::wheel::read_record_file;
|
||||
use crate::{Error, Layout};
|
||||
|
||||
/// Uninstall the wheel represented by the given `.dist-info` directory.
|
||||
pub fn uninstall_wheel(dist_info: &Path) -> Result<Uninstall, Error> {
|
||||
pub fn uninstall_wheel(
|
||||
dist_info: &Path,
|
||||
distribution: impl Display,
|
||||
layout: &Layout,
|
||||
) -> Result<Uninstall, Error> {
|
||||
let Some(site_packages) = dist_info.parent() else {
|
||||
return Err(Error::BrokenVenv(
|
||||
"dist-info directory is not in a site-packages directory".to_string(),
|
||||
@@ -40,6 +47,10 @@ pub fn uninstall_wheel(dist_info: &Path) -> Result<Uninstall, Error> {
|
||||
for entry in &record {
|
||||
let path = site_packages.join(&entry.path);
|
||||
|
||||
if !is_path_in_scheme(&entry.path, site_packages, &distribution, layout) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// On Windows, deleting the current executable is a special case.
|
||||
#[cfg(windows)]
|
||||
if let Some(itself) = itself.as_ref() {
|
||||
@@ -148,10 +159,64 @@ pub fn uninstall_wheel(dist_info: &Path) -> Result<Uninstall, Error> {
|
||||
})
|
||||
}
|
||||
|
||||
static WARNED_FOR_PACKAGE: OnceLock<Mutex<HashSet<String>>> = OnceLock::new();
|
||||
|
||||
/// Warn and reject paths that are not part of the venv or the system interpreter.
|
||||
///
|
||||
/// Reject RECORD entries that escape site-packages via path traversal (e.g.,
|
||||
/// `../../../etc/passwd`). A malicious wheel could include such entries to cause
|
||||
/// deletion of arbitrary files on uninstall.
|
||||
fn is_path_in_scheme(
|
||||
path: &str,
|
||||
site_packages: &Path,
|
||||
distribution: impl Display,
|
||||
layout: &Layout,
|
||||
) -> bool {
|
||||
let normalized = normalize_path(&site_packages.join(path));
|
||||
|
||||
// `purelib` or `platlib` are site-packages (depending on `Root-Is-Purelib`). As
|
||||
// `.data/*` goes into the directories of `scheme`, `.dist-info` goes into site-packages
|
||||
// and all other content goes into site-packages, the condition below covers all valid
|
||||
// directories, in venvs, system interpreters and custom installation schemes.
|
||||
//
|
||||
// For a venv, `data` is the venv root: A wheel can write into the entire venv through
|
||||
// `.data/data`. For a system environment, wheels are allowed to write to
|
||||
// whole system directories, for example `data` is `/usr/local` for system Python on
|
||||
// Ubuntu 24.04.
|
||||
if normalized.starts_with(&layout.scheme.data)
|
||||
|| normalized.starts_with(&layout.scheme.purelib)
|
||||
|| normalized.starts_with(&layout.scheme.platlib)
|
||||
|| normalized.starts_with(&layout.scheme.scripts)
|
||||
|| normalized.starts_with(&layout.scheme.include)
|
||||
{
|
||||
true
|
||||
} else {
|
||||
// A package that does this is malformed to the point of being a risk to the user, be
|
||||
// annoying about it, but only once per package.
|
||||
if WARNED_FOR_PACKAGE
|
||||
.get_or_init(|| Mutex::new(HashSet::new()))
|
||||
.lock()
|
||||
.expect("The mutex is broken, did some other thread panic?")
|
||||
.insert(distribution.to_string())
|
||||
{
|
||||
warn_user!(
|
||||
"Invalid RECORD entry in {} that escapes the Python environment, skipping: {}",
|
||||
distribution,
|
||||
path
|
||||
);
|
||||
}
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
/// Uninstall the egg represented by the `.egg-info` directory.
|
||||
///
|
||||
/// See: <https://github.com/pypa/pip/blob/41587f5e0017bcd849f42b314dc8a34a7db75621/src/pip/_internal/req/req_uninstall.py#L483>
|
||||
pub fn uninstall_egg(egg_info: &Path) -> Result<Uninstall, Error> {
|
||||
pub fn uninstall_egg(
|
||||
egg_info: &Path,
|
||||
distribution: impl Display,
|
||||
layout: &Layout,
|
||||
) -> Result<Uninstall, Error> {
|
||||
let mut file_count = 0usize;
|
||||
let mut dir_count = 0usize;
|
||||
|
||||
@@ -194,6 +259,10 @@ pub fn uninstall_egg(egg_info: &Path) -> Result<Uninstall, Error> {
|
||||
for entry in top_level {
|
||||
let path = dist_location.join(&entry);
|
||||
|
||||
if !is_path_in_scheme(&entry, dist_location, &distribution, layout) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Remove as a directory.
|
||||
match fs_err::remove_dir_all(&path) {
|
||||
Ok(()) => {
|
||||
@@ -347,3 +416,122 @@ fn normalize_path(path: &Path) -> PathBuf {
|
||||
}
|
||||
ret
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use assert_fs::prelude::*;
|
||||
|
||||
use uv_pypi_types::Scheme;
|
||||
|
||||
use crate::Layout;
|
||||
use crate::uninstall::{uninstall_egg, uninstall_wheel};
|
||||
|
||||
/// Uninstall must not remove files outside the install scheme.
|
||||
#[test]
|
||||
fn test_uninstall_record_path_traversal() {
|
||||
let venv = assert_fs::TempDir::new().unwrap();
|
||||
let site_packages = venv.child("lib/python3.12/site-packages");
|
||||
let outside_dir = assert_fs::TempDir::new().unwrap();
|
||||
|
||||
// Create a file outside site-packages that a malicious RECORD might target.
|
||||
let target_file = outside_dir.child("traversal_target.txt");
|
||||
target_file.write_str("I should not be deleted").unwrap();
|
||||
|
||||
// Build a relative traversal path from site-packages to the target file.
|
||||
let dist_info = site_packages.child("evilpkg-0.1.0.dist-info");
|
||||
dist_info.create_dir_all().unwrap();
|
||||
let target_path = pathdiff::diff_paths(target_file.path(), site_packages.path()).unwrap();
|
||||
assert!(site_packages.join(&target_path).exists());
|
||||
|
||||
// Add the invalid path to the RECORD.
|
||||
let record_content = format!(
|
||||
"evilpkg/__init__.py,,0\n\
|
||||
evilpkg-0.1.0.dist-info/METADATA,,0\n\
|
||||
evilpkg-0.1.0.dist-info/RECORD,,\n\
|
||||
{},,0\n",
|
||||
target_path.display()
|
||||
);
|
||||
dist_info
|
||||
.child("RECORD")
|
||||
.write_str(&record_content)
|
||||
.unwrap();
|
||||
|
||||
// Also create the legitimate files so uninstall can remove them.
|
||||
let init_py = site_packages.child("evilpkg/__init__.py");
|
||||
init_py.touch().unwrap();
|
||||
let metadata = dist_info.child("METADATA");
|
||||
metadata.touch().unwrap();
|
||||
|
||||
// Something that looks sufficiently like a Unix venv.
|
||||
let layout = Layout {
|
||||
sys_executable: venv.path().join("bin/python"),
|
||||
python_version: (3, 13),
|
||||
os_name: "posix".to_string(),
|
||||
scheme: Scheme {
|
||||
purelib: site_packages.to_path_buf(),
|
||||
platlib: site_packages.to_path_buf(),
|
||||
scripts: venv.path().join("bin"),
|
||||
data: venv.path().to_path_buf(),
|
||||
include: venv.path().join("include/python3.12"),
|
||||
},
|
||||
};
|
||||
|
||||
uninstall_wheel(dist_info.path(), "evilpkg 0.1.0", &layout).unwrap();
|
||||
|
||||
// The regular package files have been removed, while the file outside the scheme still
|
||||
// exists.
|
||||
assert!(target_file.exists());
|
||||
assert!(!metadata.exists());
|
||||
assert!(!init_py.exists());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_uninstall_egg_info_path_traversal() {
|
||||
let venv = assert_fs::TempDir::new().unwrap();
|
||||
let site_packages = venv.child("lib/python3.12/site-packages");
|
||||
let outside_dir = assert_fs::TempDir::new().unwrap();
|
||||
|
||||
// Create a directory outside site-packages that a malicious top_level.txt might target.
|
||||
let target_dir = outside_dir.child("traversal_target");
|
||||
let target_file = target_dir.child("secret.txt");
|
||||
target_file.write_str("I should not be deleted").unwrap();
|
||||
|
||||
// Build a relative traversal path from site-packages to the target directory.
|
||||
let egg_info = site_packages.child("evilpkg-0.1.0.egg-info");
|
||||
egg_info.create_dir_all().unwrap();
|
||||
let target_path = pathdiff::diff_paths(target_dir.path(), site_packages.path()).unwrap();
|
||||
assert!(site_packages.join(&target_path).exists());
|
||||
|
||||
// Create a fake egg-info directory with a top_level.txt containing a path traversal entry.
|
||||
egg_info
|
||||
.child("top_level.txt")
|
||||
.write_str(&format!("evilpkg\n{}\n", target_path.display()))
|
||||
.unwrap();
|
||||
|
||||
// Also create the legitimate package directory so uninstall can remove it.
|
||||
let init_py = site_packages.child("evilpkg").child("__init__.py");
|
||||
init_py.touch().unwrap();
|
||||
|
||||
// Something that looks sufficiently like a Unix venv.
|
||||
let layout = Layout {
|
||||
sys_executable: venv.path().join("bin/python"),
|
||||
python_version: (3, 13),
|
||||
os_name: "posix".to_string(),
|
||||
scheme: Scheme {
|
||||
purelib: site_packages.to_path_buf(),
|
||||
platlib: site_packages.to_path_buf(),
|
||||
scripts: venv.path().join("bin"),
|
||||
data: venv.path().to_path_buf(),
|
||||
include: venv.path().join("include/python3.12"),
|
||||
},
|
||||
};
|
||||
|
||||
uninstall_egg(egg_info.path(), "evilpkg 0.1.0", &layout).unwrap();
|
||||
|
||||
// The regular package directory has been removed, while the directory outside the scheme still exists.
|
||||
assert!(target_dir.exists());
|
||||
assert!(target_file.exists());
|
||||
assert!(!init_py.exists());
|
||||
assert!(!egg_info.exists());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,18 +1,23 @@
|
||||
use uv_distribution_types::{InstalledDist, InstalledDistKind, InstalledEggInfoFile};
|
||||
use uv_install_wheel::Layout;
|
||||
|
||||
/// Uninstall a package from the specified Python environment.
|
||||
pub async fn uninstall(
|
||||
dist: &InstalledDist,
|
||||
layout: &Layout,
|
||||
) -> Result<uv_install_wheel::Uninstall, UninstallError> {
|
||||
let uninstall = tokio::task::spawn_blocking({
|
||||
let dist = dist.clone();
|
||||
let layout = layout.clone();
|
||||
move || match dist.kind {
|
||||
InstalledDistKind::Registry(_) | InstalledDistKind::Url(_) => {
|
||||
Ok(uv_install_wheel::uninstall_wheel(dist.install_path())?)
|
||||
}
|
||||
InstalledDistKind::EggInfoDirectory(_) => {
|
||||
Ok(uv_install_wheel::uninstall_egg(dist.install_path())?)
|
||||
}
|
||||
InstalledDistKind::Registry(_) | InstalledDistKind::Url(_) => Ok(
|
||||
uv_install_wheel::uninstall_wheel(dist.install_path(), &dist, &layout)?,
|
||||
),
|
||||
InstalledDistKind::EggInfoDirectory(_) => Ok(uv_install_wheel::uninstall_egg(
|
||||
dist.install_path(),
|
||||
&dist,
|
||||
&layout,
|
||||
)?),
|
||||
InstalledDistKind::LegacyEditable(dist) => {
|
||||
Ok(uv_install_wheel::uninstall_legacy_editable(&dist.egg_link)?)
|
||||
}
|
||||
|
||||
+12
-12
@@ -70,8 +70,8 @@ uv-workspace = { workspace = true, features = ["clap"] }
|
||||
anstream = { workspace = true }
|
||||
anyhow = { workspace = true }
|
||||
axoupdater = { workspace = true, features = [
|
||||
"github_releases",
|
||||
"tokio",
|
||||
"github_releases",
|
||||
"tokio",
|
||||
], optional = true }
|
||||
clap = { workspace = true, features = ["derive", "string", "wrap_help"] }
|
||||
console = { workspace = true }
|
||||
@@ -175,16 +175,16 @@ tracing-durations-export = ["dep:tracing-durations-export", "uv-resolver/tracing
|
||||
|
||||
# Features that only apply when running tests, no-ops otherwise.
|
||||
test-defaults = [
|
||||
"test-crates-io",
|
||||
"test-git",
|
||||
"test-git-lfs",
|
||||
"test-pypi",
|
||||
"test-r2",
|
||||
"test-python",
|
||||
"test-python-managed",
|
||||
"test-python-eol",
|
||||
"test-slow",
|
||||
"test-ecosystem"
|
||||
"test-crates-io",
|
||||
"test-git",
|
||||
"test-git-lfs",
|
||||
"test-pypi",
|
||||
"test-r2",
|
||||
"test-python",
|
||||
"test-python-managed",
|
||||
"test-python-eol",
|
||||
"test-slow",
|
||||
"test-ecosystem"
|
||||
]
|
||||
# Introduces a testing dependency on crates.io.
|
||||
test-crates-io = []
|
||||
|
||||
@@ -806,8 +806,9 @@ async fn execute_plan(
|
||||
if !uninstalls.is_empty() {
|
||||
let start = std::time::Instant::now();
|
||||
|
||||
let layout = venv.interpreter().layout();
|
||||
for dist_info in &uninstalls {
|
||||
match uv_installer::uninstall(dist_info).await {
|
||||
match uv_installer::uninstall(dist_info, &layout).await {
|
||||
Ok(summary) => {
|
||||
debug!(
|
||||
"Uninstalled {} ({} file{}, {} director{})",
|
||||
|
||||
@@ -201,8 +201,9 @@ pub(crate) async fn pip_uninstall(
|
||||
|
||||
// Uninstall each package.
|
||||
if !dry_run.enabled() {
|
||||
let layout = environment.interpreter().layout();
|
||||
for distribution in &distributions {
|
||||
let summary = uv_installer::uninstall(distribution).await?;
|
||||
let summary = uv_installer::uninstall(distribution, &layout).await?;
|
||||
debug!(
|
||||
"Uninstalled {} ({} file{}, {} director{})",
|
||||
distribution.name(),
|
||||
|
||||
@@ -509,3 +509,73 @@ fn dry_run_uninstall_egg_info() -> Result<()> {
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Uninstall must not remove files outside the install scheme.
|
||||
///
|
||||
/// A malformed or malicious wheel can include path-traversal entries
|
||||
/// (e.g. `../../../../../etc/passwd`) in its RECORD file. During uninstall those entries are joined
|
||||
/// with the site-packages directory and could cause deletion of files outside the installation
|
||||
/// scheme.
|
||||
#[test]
|
||||
fn uninstall_record_path_traversal() -> Result<()> {
|
||||
// The traversal-depth count differs between Unix (`.venv/lib/pythonX.Y/site-packages`)
|
||||
// and Windows (`.venv/Lib/site-packages`), so normalize the `../` sequence in the warning.
|
||||
let context = uv_test::test_context!("3.12").with_filter((
|
||||
r"(\.\./)+traversal_target\.txt",
|
||||
"[..]/traversal_target.txt",
|
||||
));
|
||||
|
||||
context
|
||||
.init()
|
||||
.arg("--lib")
|
||||
.arg("evilpkg")
|
||||
.assert()
|
||||
.success();
|
||||
context.pip_install().arg("./evilpkg").assert().success();
|
||||
|
||||
// Build the relative traversal path from site-packages to a target file outside
|
||||
// site-packages but inside the test temp dir. RECORD uses forward slashes, even on
|
||||
// Windows, and the venv layout (and thus the traversal depth) differs by platform,
|
||||
// so we construct the path manually and filter the leading `../` sequence out of the
|
||||
// snapshot above.
|
||||
let target_file = context.temp_dir.child("traversal_target.txt");
|
||||
target_file.write_str("I should not be deleted")?;
|
||||
// Canonicalize the temp dir, since `site_packages` is built from a canonicalized path
|
||||
// (with `\\?\`), which would otherwise make `strip_prefix` fail.
|
||||
let canonical_temp_dir = context.temp_dir.canonicalize()?;
|
||||
let depth = context
|
||||
.site_packages()
|
||||
.strip_prefix(&canonical_temp_dir)?
|
||||
.components()
|
||||
.count();
|
||||
let traversal_record = format!("{}traversal_target.txt", "../".repeat(depth));
|
||||
|
||||
let record_file = context
|
||||
.site_packages()
|
||||
.join("evilpkg-0.1.0.dist-info/RECORD");
|
||||
let record = fs_err::read_to_string(&record_file)?;
|
||||
let record = format!("{}\n{},,0\n", record.trim(), traversal_record);
|
||||
fs_err::write(record_file, &record)?;
|
||||
|
||||
let init_py = context.site_packages().join("evilpkg/__init__.py");
|
||||
assert!(context.site_packages().join(&traversal_record).exists());
|
||||
assert!(init_py.exists());
|
||||
|
||||
uv_snapshot!(context.filters(), context.pip_uninstall()
|
||||
.arg("evilpkg"), @"
|
||||
success: true
|
||||
exit_code: 0
|
||||
----- stdout -----
|
||||
|
||||
----- stderr -----
|
||||
warning: Invalid RECORD entry in evilpkg==0.1.0 (from file://[TEMP_DIR]/evilpkg) that escapes the Python environment, skipping: [..]/traversal_target.txt
|
||||
Uninstalled 1 package in [TIME]
|
||||
- evilpkg==0.1.0 (from file://[TEMP_DIR]/evilpkg)
|
||||
");
|
||||
|
||||
// The regular package files have been removed, while the file outside the scheme still exists.
|
||||
assert!(target_file.exists());
|
||||
assert!(!init_py.exists());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user