Skip to content

Commit 726666b

Browse files
authored
[rust] Prevent path traversal in tar and pkg extraction (#17668). Reported by @Encrypter-404
* fix: prevent path traversal in tar and pkg extraction Add validation to reject archive entries containing ParentDir (..) components in uncompress_tar() and uncompress_pkg(). This prevents malicious archives from writing files outside the intended extraction directory. The shared validation logic is refactored into check_path_traversal() to avoid duplication (DRY principle). - CVE-2025-XXXX: Path Traversal via uncompress_tar - CVE-2025-YYYY: Path Traversal via uncompress_pkg * [rust] Removed the duplicate empty-path check from uncompress_tar() * [rust] Simplify unit tests for path traversal logging * [rust] Format code in files module
1 parent 8b85e9a commit 726666b

1 file changed

Lines changed: 108 additions & 1 deletion

File tree

rust/src/files.rs

Lines changed: 108 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,22 @@ pub fn create_path_if_not_exists(path: &Path) -> Result<(), Error> {
9292
Ok(())
9393
}
9494

95+
pub fn check_path_traversal(entry_path: &Path) -> Result<(), Error> {
96+
if entry_path.as_os_str().is_empty()
97+
|| entry_path.components().any(|c| {
98+
matches!(
99+
c,
100+
std::path::Component::ParentDir
101+
| std::path::Component::RootDir
102+
| std::path::Component::Prefix(_)
103+
)
104+
})
105+
{
106+
return Err(anyhow!("Unsafe entry (path traversal): {:?}", entry_path));
107+
}
108+
Ok(())
109+
}
110+
95111
pub fn uncompress(
96112
compressed_file: &str,
97113
target: &Path,
@@ -278,6 +294,7 @@ pub fn uncompress_pkg(compressed_file: &str, target: &Path, log: &Logger) -> Res
278294
while let Some(next) = cpio_reader.next() {
279295
let entry = next?;
280296
let name = entry.name();
297+
check_path_traversal(Path::new(name))?;
281298
let mut file = Vec::new();
282299
cpio_reader.read_to_end(&mut file)?;
283300
let target_path_buf = target_path.join(name);
@@ -430,7 +447,13 @@ pub fn uncompress_tar(decoder: &mut dyn Read, target: &Path, log: &Logger) -> Re
430447
let mut archive = Archive::new(Cursor::new(buffer));
431448
for entry in archive.entries()? {
432449
let mut entry_decoder = entry?;
433-
let entry_path: PathBuf = entry_decoder.path()?.iter().skip(1).collect();
450+
let path = entry_decoder.path()?;
451+
let entry_path: PathBuf = if path.iter().count() > 1 {
452+
path.iter().skip(1).collect()
453+
} else {
454+
path.to_path_buf()
455+
};
456+
check_path_traversal(&entry_path)?;
434457
let entry_target = target.join(entry_path);
435458
fs::create_dir_all(entry_target.parent().unwrap())?;
436459
entry_decoder.unpack(entry_target)?;
@@ -762,7 +785,9 @@ pub fn get_win_file_version(file_path: &str) -> Option<String> {
762785

763786
#[cfg(test)]
764787
mod tests {
788+
use super::*;
765789
use super::{PBZX_MAGIC, decode_pbzx};
790+
use std::io::Cursor;
766791
use std::io::Write;
767792
use xz2::write::XzEncoder;
768793

@@ -818,4 +843,86 @@ mod tests {
818843
fn decode_pbzx_rejects_non_pbzx_input() {
819844
assert!(decode_pbzx(b"\x1f\x8b\x08not a pbzx stream").is_err());
820845
}
846+
847+
fn build_tar(entries: &[(&str, &[u8])]) -> Vec<u8> {
848+
let mut buffer = Vec::new();
849+
for (name, contents) in entries {
850+
let mut header = tar::Header::new_gnu();
851+
header.set_size(contents.len() as u64);
852+
header.set_mode(0o644);
853+
header.set_path("browser/file.txt").unwrap();
854+
header.set_cksum();
855+
856+
let mut header_bytes = header.as_bytes().to_vec();
857+
let name_bytes = name.as_bytes();
858+
assert!(name_bytes.len() <= 100, "test tar name too long");
859+
header_bytes[0..100].fill(0);
860+
header_bytes[0..name_bytes.len()].copy_from_slice(name_bytes);
861+
header_bytes[148..156].fill(b' ');
862+
let checksum: u32 = header_bytes.iter().map(|byte| *byte as u32).sum();
863+
let checksum_bytes = format!("{:06o}\0 ", checksum);
864+
header_bytes[148..156].copy_from_slice(checksum_bytes.as_bytes());
865+
866+
buffer.extend_from_slice(&header_bytes);
867+
buffer.extend_from_slice(contents);
868+
869+
let remainder = contents.len() % 512;
870+
if remainder != 0 {
871+
buffer.extend_from_slice(&vec![0u8; 512 - remainder]);
872+
}
873+
}
874+
buffer.extend_from_slice(&[0u8; 1024]);
875+
buffer
876+
}
877+
878+
#[test]
879+
fn check_path_traversal_allows_safe_paths() {
880+
assert!(check_path_traversal(Path::new("browser/file.txt")).is_ok());
881+
}
882+
883+
#[test]
884+
fn check_path_traversal_rejects_empty_path() {
885+
let err = check_path_traversal(Path::new("")).unwrap_err();
886+
assert!(err.to_string().contains("Unsafe entry (path traversal)"));
887+
}
888+
889+
#[test]
890+
fn uncompress_tar_extracts_safe_entry() {
891+
let temp_dir = tempfile::tempdir().unwrap();
892+
let target = temp_dir.path().join("extract");
893+
let tar_data = build_tar(&[("browser/file.txt", b"hello")]);
894+
let mut decoder = Cursor::new(tar_data);
895+
let log = Logger::new();
896+
897+
uncompress_tar(&mut decoder, &target, &log).unwrap();
898+
899+
assert_eq!(fs::read(target.join("file.txt")).unwrap(), b"hello");
900+
}
901+
902+
#[test]
903+
fn uncompress_tar_keeps_single_component_entry() {
904+
let temp_dir = tempfile::tempdir().unwrap();
905+
let target = temp_dir.path().join("extract");
906+
let tar_data = build_tar(&[("file.txt", b"hello")]);
907+
let mut decoder = Cursor::new(tar_data);
908+
let log = Logger::new();
909+
910+
uncompress_tar(&mut decoder, &target, &log).unwrap();
911+
912+
assert_eq!(fs::read(target.join("file.txt")).unwrap(), b"hello");
913+
}
914+
915+
#[test]
916+
fn uncompress_tar_rejects_path_traversal_entry() {
917+
let temp_dir = tempfile::tempdir().unwrap();
918+
let target = temp_dir.path().join("extract");
919+
let escape_path = temp_dir.path().join("escape.txt");
920+
let tar_data = build_tar(&[("browser/../../escape.txt", b"owned")]);
921+
let mut decoder = Cursor::new(tar_data);
922+
let log = Logger::new();
923+
924+
let err = uncompress_tar(&mut decoder, &target, &log).unwrap_err();
925+
assert!(err.to_string().contains("Unsafe entry (path traversal)"));
926+
assert!(!escape_path.exists());
927+
}
821928
}

0 commit comments

Comments
 (0)