Skip to content

Commit 095a075

Browse files
committed
fix: follow symlink chains with hop limit, improve test naming and coverage
Signed-off-by: danbugs <danilochiarlone@gmail.com>
1 parent 2ccf3a9 commit 095a075

1 file changed

Lines changed: 55 additions & 39 deletions

File tree

host/src/lib.rs

Lines changed: 55 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -767,34 +767,40 @@ impl FsSandbox {
767767
let mut out = resolved_ancestor;
768768
for name in tail.into_iter().rev() {
769769
out.push(name);
770-
// Catch symlinks in the non-existing tail — including dangling ones.
771-
if let Ok(meta) = std::fs::symlink_metadata(&out) {
772-
if meta.file_type().is_symlink() {
773-
let target = std::fs::read_link(&out)?;
774-
let abs = if target.is_absolute() {
775-
target
776-
} else {
777-
out.parent().unwrap_or(&self.root).join(&target)
778-
};
779-
// Normalise the absolute target (resolve .. etc) without
780-
// touching the filesystem, then check the prefix.
781-
let mut norm = std::path::PathBuf::new();
782-
for c in abs.components() {
783-
match c {
784-
std::path::Component::ParentDir => {
785-
norm.pop();
786-
}
787-
std::path::Component::CurDir => {}
788-
c => norm.push(c),
770+
// Walk the symlink chain (with hop limit) to catch escapes
771+
// through dangling or chained symlinks.
772+
const MAX_SYMLINK_HOPS: usize = 40;
773+
let mut cursor = out.clone();
774+
for _ in 0..MAX_SYMLINK_HOPS {
775+
let Ok(meta) = std::fs::symlink_metadata(&cursor) else {
776+
break;
777+
};
778+
if !meta.file_type().is_symlink() {
779+
break;
780+
}
781+
let target = std::fs::read_link(&cursor)?;
782+
let abs = if target.is_absolute() {
783+
target
784+
} else {
785+
cursor.parent().unwrap_or(&self.root).join(&target)
786+
};
787+
let mut norm = std::path::PathBuf::new();
788+
for c in abs.components() {
789+
match c {
790+
std::path::Component::ParentDir => {
791+
norm.pop();
789792
}
793+
std::path::Component::CurDir => {}
794+
c => norm.push(c),
790795
}
791-
if !norm.starts_with(&self.root) {
792-
return Err(anyhow!(
793-
"symlink target escapes mount root: {:?}",
794-
guest_path
795-
));
796-
}
797796
}
797+
if !norm.starts_with(&self.root) {
798+
return Err(anyhow!(
799+
"symlink target escapes mount root: {:?}",
800+
guest_path
801+
));
802+
}
803+
cursor = norm;
798804
}
799805
}
800806
Ok(out)
@@ -2519,11 +2525,11 @@ mod tests {
25192525

25202526
#[cfg(unix)]
25212527
#[test]
2522-
fn test_resolve_dangling_symlink_escape() {
2528+
fn resolve_rejects_dangling_symlink_escape() {
25232529
use std::os::unix::fs::symlink;
25242530
let root = tmpdir("dangling-escape");
2525-
// Create a dangling symlink whose target is outside root.
2526-
symlink("/tmp/nonexistent-outside-root", root.join("bad_link")).unwrap();
2531+
let outside = tmpdir("dangling-escape-out");
2532+
symlink(outside.join("nonexistent"), root.join("bad_link")).unwrap();
25272533
let fs_sb = FsSandbox::new(&root).unwrap();
25282534
let err = fs_sb.resolve("bad_link").unwrap_err().to_string();
25292535
assert!(
@@ -2534,14 +2540,13 @@ mod tests {
25342540

25352541
#[cfg(unix)]
25362542
#[test]
2537-
fn test_resolve_valid_internal_symlink() {
2543+
fn resolve_allows_valid_internal_symlink() {
25382544
use std::os::unix::fs::symlink;
25392545
let root = tmpdir("valid-internal");
25402546
fs::write(root.join("real_file.txt"), "hello").unwrap();
25412547
symlink(root.join("real_file.txt"), root.join("good_link")).unwrap();
25422548
let fs_sb = FsSandbox::new(&root).unwrap();
25432549
let resolved = fs_sb.resolve("good_link").unwrap();
2544-
// The symlink target is inside root, so resolve should succeed.
25452550
assert!(
25462551
resolved.starts_with(&root),
25472552
"expected path under root, got: {resolved:?}"
@@ -2550,14 +2555,11 @@ mod tests {
25502555

25512556
#[cfg(unix)]
25522557
#[test]
2553-
fn test_resolve_dangling_symlink_inside_root() {
2558+
fn resolve_allows_dangling_symlink_inside_root() {
25542559
use std::os::unix::fs::symlink;
25552560
let root = tmpdir("dangling-inside");
2556-
// Dangling symlink whose target is inside root (just doesn't exist yet).
25572561
symlink(root.join("future_file.txt"), root.join("ok_link")).unwrap();
25582562
let fs_sb = FsSandbox::new(&root).unwrap();
2559-
// This should be allowed — the target is within root even though it
2560-
// doesn't exist yet.
25612563
let resolved = fs_sb.resolve("ok_link").unwrap();
25622564
assert!(
25632565
resolved.starts_with(&root),
@@ -2567,17 +2569,31 @@ mod tests {
25672569

25682570
#[cfg(unix)]
25692571
#[test]
2570-
fn test_resolve_symlink_chain_escape() {
2572+
fn resolve_rejects_symlink_chain_escape() {
25712573
use std::os::unix::fs::symlink;
25722574
let root = tmpdir("chain-escape");
25732575
let outside = tmpdir("chain-outside");
2574-
// symlink B -> outside (dangling or not, target is outside root)
25752576
symlink(&outside, root.join("link_b")).unwrap();
2576-
// symlink A -> link_b
25772577
symlink(root.join("link_b"), root.join("link_a")).unwrap();
25782578
let fs_sb = FsSandbox::new(&root).unwrap();
2579-
// Resolving link_a should fail because it chains through link_b
2580-
// which points outside root.
2579+
let err = fs_sb.resolve("link_a").unwrap_err().to_string();
2580+
assert!(
2581+
err.contains("escapes mount root"),
2582+
"expected escape error, got: {err}"
2583+
);
2584+
}
2585+
2586+
#[cfg(unix)]
2587+
#[test]
2588+
fn resolve_rejects_chained_dangling_symlink_escape() {
2589+
use std::os::unix::fs::symlink;
2590+
let root = tmpdir("chain-dangling");
2591+
let outside = tmpdir("chain-dangling-out");
2592+
// link_b -> dangling path outside root
2593+
symlink(outside.join("nonexistent"), root.join("link_b")).unwrap();
2594+
// link_a -> link_b (which is under root, but chains outside)
2595+
symlink(root.join("link_b"), root.join("link_a")).unwrap();
2596+
let fs_sb = FsSandbox::new(&root).unwrap();
25812597
let err = fs_sb.resolve("link_a").unwrap_err().to_string();
25822598
assert!(
25832599
err.contains("escapes mount root"),

0 commit comments

Comments
 (0)