Skip to content

tar: resolve symlink resilience and path normalization issues #323

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kaladron wants to merge 1 commit into
base: main
Choose a base branch
from
+403 −52
Open

tar: resolve symlink resilience and path normalization issues #323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
195 changes: 143 additions & 52 deletions src/uu/tar/src/operations/create.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,16 @@ use std::path::{self, Path, PathBuf};
use tar::Builder;
use uucore::error::UResult;

/// Create a tar archive from the specified files
/// Create a tar archive from the specified files.
///
/// # Arguments
///
/// * `output` - Destination where the tar archive should be written
/// * `files` - Slice of file paths to add to the archive
/// * `allow_absolute` - Allow absolute paths while creating archive
/// * `verbose` - Whether to print verbose output during creation
/// * `output` - Destination where the tar archive data should be written.
/// * `status_output` - Writer for verbose progress output (e.g. stderr/stdout).
/// * `files` - Slice of file paths to add to the archive.
/// * `allow_absolute` - Allow absolute paths while creating archive.
/// * `verbose` - Whether to print verbose output during creation.
/// * `compression` - The compression mode to apply to the output.
///
/// # Errors
///
Expand All @@ -44,16 +46,21 @@ pub fn create_archive(
let writer = ArchiveWriter::new(output, compression)?;
let mut builder = Builder::new(writer);
builder.preserve_absolute(allow_absolute);
builder.follow_symlinks(false);

// Add each file or directory to the archive
for &path in files {
// Check if path exists
if !path.exists() {
return Err(TarError::FileNotFound {
path: path.to_path_buf(),
// Check if path exists (without following symlinks)
let metadata = match path.symlink_metadata() {
Ok(m) => m,
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
return Err(TarError::FileNotFound {
path: path.to_path_buf(),
}
.into());
}
.into());
}
Err(e) => return Err(TarError::Io(e).into()),
};

if verbose {
print_verbose_tree(&mut status_output, path)?;
Expand All @@ -62,15 +69,15 @@ pub fn create_archive(
let normalized_name = get_normalized_path(path, allow_absolute)?;

// If it's a directory, recursively add all contents
if path.is_dir() {
if metadata.is_dir() {
builder.append_dir_all(normalized_name, path).map_err(|e| {
TarError::CannotAddDirectory {
path: path.to_path_buf(),
source: e,
}
})?;
} else {
// For files, add them directly
// For files/symlinks, add them directly
builder
.append_path_with_name(path, normalized_name)
.map_err(|e| TarError::CannotAddFile {
Expand All @@ -90,36 +97,78 @@ pub fn create_archive(
Ok(())
}

/// Prints the list of files in `path` recursively in a verbose format.
///
/// Traverses the directory tree at `path` and writes each entry name to
/// `status_output`. Directory entries are printed with a trailing slash.
/// Does not follow symlinks.
fn print_verbose_tree(status_output: &mut impl Write, path: &Path) -> UResult<()> {
let to_print = get_tree(path)?
.iter()
.map(|p| (p.is_dir(), p.display().to_string()))
.map(|(is_dir, path)| {
if is_dir {
format!("{}{}", path, path::MAIN_SEPARATOR)
} else {
path
}
})
.collect::<Vec<_>>()
.join("\n");
writeln!(status_output, "{to_print}").map_err(TarError::Io)?;
for (p, is_dir) in get_tree(path)? {
if is_dir {
writeln!(status_output, "{}{}", p.display(), path::MAIN_SEPARATOR)
.map_err(TarError::Io)?;
} else {
writeln!(status_output, "{}", p.display()).map_err(TarError::Io)?;
}
}
Ok(())
}

fn needs_cleaning(path: &Path, allow_absolute: bool) -> bool {
for component in path.components() {
match component {
Prefix(_) | RootDir => {
if !allow_absolute {
return true;
}
}
Component::CurDir | ParentDir => return true,
Component::Normal(_) => {}
}
}
false
}

/// Normalizes the path for archiving and displays a warning if leading components are stripped.
///
/// Lexically cleans the path first (resolving `.` and `..`). If `allow_absolute`
/// is false, it strips leading absolute prefixes (`RootDir`, `Prefix`) and
/// leading `ParentDir` (`..`) components. Prints a warning to stderr if any
/// prefix components were removed.
fn get_normalized_path(path: &Path, allow_absolute: bool) -> Result<PathBuf, TarError> {
if let Some(normalized) = normalize_path(path, allow_absolute) {
let original_components: Vec<Component> = path.components().collect();
let normalized_components: Vec<Component> = normalized.components().collect();
if original_components.len() > normalized_components.len() {
let removed: PathBuf = original_components
[..original_components.len() - normalized_components.len()]
.iter()
.collect();
if !needs_cleaning(path, allow_absolute) {
return Ok(path.to_path_buf());
}
let cleaned = clean_path(path);
let mut normalized = cleaned.clone();
let mut prefix_removed = PathBuf::new();
let mut changed = cleaned != path;

if !allow_absolute {
let mut remaining = PathBuf::new();
let mut stripped_any = false;
let mut in_leading = true;
for c in cleaned.components() {
if in_leading && matches!(c, RootDir | Prefix(_) | ParentDir) {
stripped_any = true;
prefix_removed.push(c.as_os_str());
} else {
in_leading = false;
remaining.push(c.as_os_str());
}
}
if stripped_any {
normalized = remaining;
changed = true;
}
}

if changed {
if !prefix_removed.as_os_str().is_empty() {
writeln!(
std::io::stderr(),
"tar: Removing leading `{}' from member names",
removed.display()
prefix_removed.display()
)?;
}
Ok(normalized)
Expand All @@ -128,33 +177,75 @@ fn get_normalized_path(path: &Path, allow_absolute: bool) -> Result<PathBuf, Tar
}
}

fn get_tree(path: &Path) -> Result<Vec<PathBuf>, std::io::Error> {
/// Recursively gathers all paths within `path` without following symlinks.
///
/// Returns a list of tuples containing the path and a boolean indicating
/// whether the path is a directory. Using `symlink_metadata` and
/// `DirEntry::file_type` prevents recursing into symlinked directories.
fn get_tree(path: &Path) -> Result<Vec<(PathBuf, bool)>, std::io::Error> {
let mut paths = Vec::new();
let mut stack = VecDeque::new();
stack.push_back(path.to_path_buf());

while let Some(current) = stack.pop_back() {
paths.push(current.clone());
if current.is_dir() {
for entry in fs::read_dir(current)? {
let child = entry?.path();
stack.push_back(child);

let root_metadata = path.symlink_metadata()?;
let root_is_dir = root_metadata.is_dir();
stack.push_back((path.to_path_buf(), root_is_dir));

while let Some((current, is_dir)) = stack.pop_back() {
if is_dir {
for entry in fs::read_dir(&current)? {
let entry = entry?;
let child = entry.path();
let file_type = entry.file_type()?;
let child_is_dir = file_type.is_dir();
stack.push_back((child, child_is_dir));
}
}
paths.push((current, is_dir));
}

Ok(paths)
}

fn normalize_path(path: &Path, allow_absolute: bool) -> Option<PathBuf> {
if path.is_absolute() && !allow_absolute {
Some(
path.components()
.filter(|c| !matches!(c, RootDir | ParentDir | Prefix(_)))
.collect::<PathBuf>(),
)
/// Performs lexical cleaning of a path.
///
/// Resolves `.` and `..` components where possible without accessing the
/// actual filesystem. This is used to normalize paths safely and prevent
/// equivalent path confusion.
fn clean_path(path: &Path) -> PathBuf {
let mut clean = PathBuf::new();
for component in path.components() {
match component {
Prefix(_) | RootDir => {
clean.push(component.as_os_str());
}
Component::CurDir => {}
ParentDir => {
if let Some(last) = clean.components().next_back() {
match last {
Component::Normal(_) => {
clean.pop();
}
RootDir | Prefix(_) => {
// Cannot go above root/prefix
}
ParentDir => {
clean.push(component);
}
Component::CurDir => unreachable!(),
}
} else {
clean.push(component);
}
}
Component::Normal(c) => {
clean.push(c);
}
}
}
if clean.as_os_str().is_empty() {
PathBuf::from(".")
} else {
None
clean
}
}

Expand Down
18 changes: 18 additions & 0 deletions src/uu/tar/src/operations/create_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,21 @@ fn test_create_archive_missing_file_fails() {
.unwrap_err();
assert!(err.to_string().contains("missing.txt"));
}

#[test]
fn test_clean_path() {
// Empty/relative to current dir cases
assert_eq!(clean_path(Path::new(".")), PathBuf::from("."));
assert_eq!(clean_path(Path::new("a/..")), PathBuf::from("."));
assert_eq!(clean_path(Path::new("a/b/../..")), PathBuf::from("."));
assert_eq!(clean_path(Path::new("")), PathBuf::from("."));

// Root/Prefix limit cases (cannot go above root)
assert_eq!(clean_path(Path::new("/..")), PathBuf::from("/"));
assert_eq!(clean_path(Path::new("/a/../../b")), PathBuf::from("/b"));

// Parent dir preservation cases
assert_eq!(clean_path(Path::new("../..")), PathBuf::from("../.."));
assert_eq!(clean_path(Path::new("../../a")), PathBuf::from("../../a"));
assert_eq!(clean_path(Path::new("a/../../b")), PathBuf::from("../b"));
}
Loading
Loading