Skip to content

Commit c8ba172

Browse files
committed
fix(core): use component-aware comparison in macOS/Windows containment check
paths_start_with() compared paths as raw lowercased strings on macOS and Windows, so a sibling directory sharing the destination root's name as a string prefix (e.g. /tmp/destevil vs /tmp/dest) was incorrectly treated as contained within it. The non-macOS Unix arm was unaffected since it already used Path::starts_with, which is component-aware. Replace the macOS/Windows arm with a component-wise, case-insensitive comparison matching the same semantics. Add unit tests covering the sibling bypass, genuine subdirectories, case-insensitivity, and the base-longer- than-path short-circuit branch, plus an end-to-end regression test driving the attack through a real on-disk symlink. Fixes GHSA-wcmx-7f9h-5mv5.
1 parent 69c81be commit c8ba172

4 files changed

Lines changed: 138 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Security
11+
12+
- **Case-insensitive containment check accepted a sibling of the destination root sharing a name prefix on macOS/Windows (GHSA-wcmx-7f9h-5mv5)**:
13+
`paths_start_with` (`crates/exarch-core/src/types/safe_path.rs`), used by `SafePath`'s macOS/Windows
14+
containment check, compared paths as raw lowercased strings, so `/tmp/destevil` was wrongly treated as
15+
contained within `/tmp/dest` (`"...destevil".starts_with("...dest")` is true with no component
16+
boundary). The check now compares `Path::components()` pairwise, case-folding each segment, matching
17+
the already-correct non-macOS Unix behavior except case-insensitively.
18+
1019
## [0.6.0] - 2026-08-04
1120

1221
### Security

crates/exarch-core/src/types/safe_path.rs

Lines changed: 71 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -343,12 +343,25 @@ impl SafePath {
343343
///
344344
/// On case-insensitive filesystems (macOS, Windows), attackers can bypass
345345
/// path validation using different case (e.g., `../../USERS/victim/.ssh/`).
346+
///
347+
/// Compares `path` and `base` component-by-component rather than as raw
348+
/// strings, so a shared name prefix without a component boundary (e.g. base
349+
/// `/tmp/dest` against sibling `/tmp/destevil`) is correctly rejected. See
350+
/// GHSA-wcmx-7f9h-5mv5.
346351
#[cfg(any(target_os = "macos", target_os = "windows"))]
347352
fn paths_start_with(path: &Path, base: &Path) -> bool {
348-
// Convert both paths to lowercase strings for comparison
349-
let path_str = path.to_string_lossy().to_lowercase();
350-
let base_str = base.to_string_lossy().to_lowercase();
351-
path_str.starts_with(&base_str)
353+
let mut path_components = path.components();
354+
for base_component in base.components() {
355+
let Some(path_component) = path_components.next() else {
356+
return false;
357+
};
358+
let path_str = path_component.as_os_str().to_string_lossy().to_lowercase();
359+
let base_str = base_component.as_os_str().to_string_lossy().to_lowercase();
360+
if path_str != base_str {
361+
return false;
362+
}
363+
}
364+
true
352365
}
353366

354367
/// Case-sensitive path prefix check for Unix (not macOS).
@@ -1202,4 +1215,58 @@ mod tests {
12021215
);
12031216
}
12041217
}
1218+
1219+
// --- Tests for GHSA-wcmx-7f9h-5mv5 (case-insensitive prefix bypass) ---
1220+
1221+
#[test]
1222+
#[cfg(any(target_os = "macos", target_os = "windows"))]
1223+
fn test_paths_start_with_rejects_sibling_sharing_name_prefix_ghsa_wcmx() {
1224+
// GHSA-wcmx-7f9h-5mv5: the previous implementation compared paths as
1225+
// raw strings, so `/tmp/destevil`.starts_with(`/tmp/dest`) was true
1226+
// even though `destevil` is a sibling directory, not a subdirectory
1227+
// of `dest`.
1228+
let base = Path::new("/tmp/dest");
1229+
let sibling = Path::new("/tmp/destevil");
1230+
assert!(
1231+
!paths_start_with(sibling, base),
1232+
"sibling directory sharing a name prefix must not be treated as contained"
1233+
);
1234+
}
1235+
1236+
#[test]
1237+
#[cfg(any(target_os = "macos", target_os = "windows"))]
1238+
fn test_paths_start_with_accepts_genuine_subdirectory() {
1239+
let base = Path::new("/tmp/dest");
1240+
let child = Path::new("/tmp/dest/sub");
1241+
assert!(
1242+
paths_start_with(child, base),
1243+
"genuine subdirectory must still be accepted as contained"
1244+
);
1245+
}
1246+
1247+
#[test]
1248+
#[cfg(any(target_os = "macos", target_os = "windows"))]
1249+
fn test_paths_start_with_is_case_insensitive() {
1250+
let base = Path::new("/tmp/dest");
1251+
let child = Path::new("/tmp/Dest/sub");
1252+
assert!(
1253+
paths_start_with(child, base),
1254+
"differing-case containment must still be accepted on case-insensitive filesystems"
1255+
);
1256+
}
1257+
1258+
#[test]
1259+
#[cfg(any(target_os = "macos", target_os = "windows"))]
1260+
fn test_paths_start_with_rejects_when_base_has_more_components_than_path() {
1261+
// Regression for a `break`-instead-of-`return false` mutant: when
1262+
// `base` has more components than `path`, the `path` component
1263+
// iterator is exhausted mid-loop and the function must reject
1264+
// immediately, not fall through the loop to the trailing `true`.
1265+
let base = Path::new("/tmp/dest/sub/extra");
1266+
let shorter = Path::new("/tmp/dest/sub");
1267+
assert!(
1268+
!paths_start_with(shorter, base),
1269+
"a path shorter than base must never be treated as containing base"
1270+
);
1271+
}
12051272
}

crates/exarch-core/tests/security/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
mod cve_regression;
44
mod hardlink_quota_bypass;
55
mod partial_report_skip_and_fail;
6+
mod safe_path_ghsa_wcmx;
67
mod sevenz_traversal;
78
mod symlink_target_validation;
89
mod tar_budget_parity;
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
//! End-to-end regression test for GHSA-wcmx-7f9h-5mv5: on case-insensitive
2+
//! filesystems (macOS, Windows), `SafePath`'s containment check compared a
3+
//! canonicalized parent directory against the destination root as a raw
4+
//! string prefix, so a sibling of the destination root sharing a name prefix
5+
//! (e.g. `dest` vs. `destevil`) was wrongly treated as contained within
6+
//! `dest`.
7+
//!
8+
//! This drives the attack through a real, pre-existing on-disk symlink so
9+
//! the containment check runs against an actually canonicalized path — the
10+
//! same code path a crafted archive entry resolving through a symlinked
11+
//! directory would take — rather than a synthetic `Path` value.
12+
13+
#![allow(clippy::unwrap_used)]
14+
15+
use exarch_core::ArchiveError;
16+
use exarch_core::DestDir;
17+
use exarch_core::SafePath;
18+
use exarch_core::SecurityConfig;
19+
use std::assert_matches;
20+
use std::path::PathBuf;
21+
use tempfile::TempDir;
22+
23+
#[test]
24+
#[cfg(unix)]
25+
fn ghsa_wcmx_sibling_reached_via_symlink_is_rejected() {
26+
use std::os::unix::fs::symlink;
27+
28+
let temp = TempDir::new().unwrap();
29+
30+
// Destination root: <temp>/dest
31+
let dest_root = temp.path().join("dest");
32+
std::fs::create_dir(&dest_root).unwrap();
33+
let dest = DestDir::new(dest_root.clone()).unwrap();
34+
35+
// Pre-existing sibling directory that shares `dest_root`'s name as a
36+
// string prefix but is NOT a subdirectory of it: <temp>/destevil.
37+
let sibling = temp.path().join("destevil");
38+
std::fs::create_dir(&sibling).unwrap();
39+
40+
// A symlink inside `dest` resolving to the sibling, so the validated
41+
// entry's canonicalized parent is the sibling directory rather than
42+
// `dest_root` itself.
43+
let escape_link = dest_root.join("escape");
44+
symlink(&sibling, &escape_link).unwrap();
45+
46+
let config = SecurityConfig::default().validate().unwrap();
47+
let entry_path = PathBuf::from("escape/secret.txt");
48+
49+
let result = SafePath::validate(&entry_path, &dest, &config);
50+
51+
assert_matches!(
52+
result,
53+
Err(ArchiveError::PathTraversal { .. }),
54+
"sibling directory reached via symlink and sharing a name prefix with \
55+
the destination root must be rejected, got: {result:?}"
56+
);
57+
}

0 commit comments

Comments
 (0)