Skip to content

Commit 1397275

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

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
@@ -762,34 +762,40 @@ impl FsSandbox {
762762
let mut out = resolved_ancestor;
763763
for name in tail.into_iter().rev() {
764764
out.push(name);
765-
// Catch symlinks in the non-existing tail — including dangling ones.
766-
if let Ok(meta) = std::fs::symlink_metadata(&out) {
767-
if meta.file_type().is_symlink() {
768-
let target = std::fs::read_link(&out)?;
769-
let abs = if target.is_absolute() {
770-
target
771-
} else {
772-
out.parent().unwrap_or(&self.root).join(&target)
773-
};
774-
// Normalise the absolute target (resolve .. etc) without
775-
// touching the filesystem, then check the prefix.
776-
let mut norm = std::path::PathBuf::new();
777-
for c in abs.components() {
778-
match c {
779-
std::path::Component::ParentDir => {
780-
norm.pop();
781-
}
782-
std::path::Component::CurDir => {}
783-
c => norm.push(c),
765+
// Walk the symlink chain (with hop limit) to catch escapes
766+
// through dangling or chained symlinks.
767+
const MAX_SYMLINK_HOPS: usize = 40;
768+
let mut cursor = out.clone();
769+
for _ in 0..MAX_SYMLINK_HOPS {
770+
let Ok(meta) = std::fs::symlink_metadata(&cursor) else {
771+
break;
772+
};
773+
if !meta.file_type().is_symlink() {
774+
break;
775+
}
776+
let target = std::fs::read_link(&cursor)?;
777+
let abs = if target.is_absolute() {
778+
target
779+
} else {
780+
cursor.parent().unwrap_or(&self.root).join(&target)
781+
};
782+
let mut norm = std::path::PathBuf::new();
783+
for c in abs.components() {
784+
match c {
785+
std::path::Component::ParentDir => {
786+
norm.pop();
784787
}
788+
std::path::Component::CurDir => {}
789+
c => norm.push(c),
785790
}
786-
if !norm.starts_with(&self.root) {
787-
return Err(anyhow!(
788-
"symlink target escapes mount root: {:?}",
789-
guest_path
790-
));
791-
}
792791
}
792+
if !norm.starts_with(&self.root) {
793+
return Err(anyhow!(
794+
"symlink target escapes mount root: {:?}",
795+
guest_path
796+
));
797+
}
798+
cursor = norm;
793799
}
794800
}
795801
Ok(out)
@@ -2514,11 +2520,11 @@ mod tests {
25142520

25152521
#[cfg(unix)]
25162522
#[test]
2517-
fn test_resolve_dangling_symlink_escape() {
2523+
fn resolve_rejects_dangling_symlink_escape() {
25182524
use std::os::unix::fs::symlink;
25192525
let root = tmpdir("dangling-escape");
2520-
// Create a dangling symlink whose target is outside root.
2521-
symlink("/tmp/nonexistent-outside-root", root.join("bad_link")).unwrap();
2526+
let outside = tmpdir("dangling-escape-out");
2527+
symlink(outside.join("nonexistent"), root.join("bad_link")).unwrap();
25222528
let fs_sb = FsSandbox::new(&root).unwrap();
25232529
let err = fs_sb.resolve("bad_link").unwrap_err().to_string();
25242530
assert!(
@@ -2529,14 +2535,13 @@ mod tests {
25292535

25302536
#[cfg(unix)]
25312537
#[test]
2532-
fn test_resolve_valid_internal_symlink() {
2538+
fn resolve_allows_valid_internal_symlink() {
25332539
use std::os::unix::fs::symlink;
25342540
let root = tmpdir("valid-internal");
25352541
fs::write(root.join("real_file.txt"), "hello").unwrap();
25362542
symlink(root.join("real_file.txt"), root.join("good_link")).unwrap();
25372543
let fs_sb = FsSandbox::new(&root).unwrap();
25382544
let resolved = fs_sb.resolve("good_link").unwrap();
2539-
// The symlink target is inside root, so resolve should succeed.
25402545
assert!(
25412546
resolved.starts_with(&root),
25422547
"expected path under root, got: {resolved:?}"
@@ -2545,14 +2550,11 @@ mod tests {
25452550

25462551
#[cfg(unix)]
25472552
#[test]
2548-
fn test_resolve_dangling_symlink_inside_root() {
2553+
fn resolve_allows_dangling_symlink_inside_root() {
25492554
use std::os::unix::fs::symlink;
25502555
let root = tmpdir("dangling-inside");
2551-
// Dangling symlink whose target is inside root (just doesn't exist yet).
25522556
symlink(root.join("future_file.txt"), root.join("ok_link")).unwrap();
25532557
let fs_sb = FsSandbox::new(&root).unwrap();
2554-
// This should be allowed — the target is within root even though it
2555-
// doesn't exist yet.
25562558
let resolved = fs_sb.resolve("ok_link").unwrap();
25572559
assert!(
25582560
resolved.starts_with(&root),
@@ -2562,17 +2564,31 @@ mod tests {
25622564

25632565
#[cfg(unix)]
25642566
#[test]
2565-
fn test_resolve_symlink_chain_escape() {
2567+
fn resolve_rejects_symlink_chain_escape() {
25662568
use std::os::unix::fs::symlink;
25672569
let root = tmpdir("chain-escape");
25682570
let outside = tmpdir("chain-outside");
2569-
// symlink B -> outside (dangling or not, target is outside root)
25702571
symlink(&outside, root.join("link_b")).unwrap();
2571-
// symlink A -> link_b
25722572
symlink(root.join("link_b"), root.join("link_a")).unwrap();
25732573
let fs_sb = FsSandbox::new(&root).unwrap();
2574-
// Resolving link_a should fail because it chains through link_b
2575-
// which points outside root.
2574+
let err = fs_sb.resolve("link_a").unwrap_err().to_string();
2575+
assert!(
2576+
err.contains("escapes mount root"),
2577+
"expected escape error, got: {err}"
2578+
);
2579+
}
2580+
2581+
#[cfg(unix)]
2582+
#[test]
2583+
fn resolve_rejects_chained_dangling_symlink_escape() {
2584+
use std::os::unix::fs::symlink;
2585+
let root = tmpdir("chain-dangling");
2586+
let outside = tmpdir("chain-dangling-out");
2587+
// link_b -> dangling path outside root
2588+
symlink(outside.join("nonexistent"), root.join("link_b")).unwrap();
2589+
// link_a -> link_b (which is under root, but chains outside)
2590+
symlink(root.join("link_b"), root.join("link_a")).unwrap();
2591+
let fs_sb = FsSandbox::new(&root).unwrap();
25762592
let err = fs_sb.resolve("link_a").unwrap_err().to_string();
25772593
assert!(
25782594
err.contains("escapes mount root"),

0 commit comments

Comments
 (0)