Skip to content

Commit a8c9321

Browse files
authored
Fix trailing-slash behavior with O_NOFOLLOW. (#419)
When a path has a trailing slash, the path component just before the trailing slash is not considered a final path component for the purposes of the OS `O_NOFOLLOW` flag. Fix cap-primitives to avoid re-appending a trailing slash when resolving paths with a trailing slash so that it doesn't inadvertently disable `O_NOFOLLOW`. This fixes GHSA-hp8f-xmx4-4qrg.
1 parent b06b76f commit a8c9321

2 files changed

Lines changed: 98 additions & 12 deletions

File tree

‎cap-primitives/src/fs/manually/open.rs‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -222,23 +222,12 @@ impl<'start> Context<'start> {
222222
dir_options()
223223
};
224224

225-
// If the last path component ended in a slash, re-add the slash,
226-
// as Rust's `Path` will have removed it, and we need it to get the
227-
// same behavior from the OS.
228-
let use_path: Cow<OsStr> = if self.components.is_empty() && self.trailing_slash {
229-
let mut p = one.to_os_string();
230-
p.push("/");
231-
Cow::Owned(p)
232-
} else {
233-
Cow::Borrowed(one)
234-
};
235-
236225
let dir_required = self.dir_required || use_options.dir_required;
237226

238227
#[allow(clippy::redundant_clone)]
239228
match open_unchecked(
240229
&self.base,
241-
use_path.as_ref(),
230+
one.as_ref(),
242231
use_options
243232
.clone()
244233
.follow(FollowSymlinks::No)

‎tests/fs_additional.rs‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#[macro_use]
66
mod sys_common;
77

8+
use cap_std::ambient_authority;
89
use cap_std::fs::{Dir, DirBuilder, OpenOptions};
910
use cap_std::time::SystemClock;
1011
use std::io::{self, Read, Write};
@@ -1412,3 +1413,99 @@ fn statat_slash() {
14121413
);
14131414
}
14141415
}
1416+
1417+
/// Test interactions between symlinks and trailing slashes.
1418+
#[test]
1419+
fn trailing_slash_symlink() {
1420+
let tmpdir = tmpdir();
1421+
1422+
check!(tmpdir.create_dir("sandbox"));
1423+
check!(symlink_dir("../outside", &tmpdir, "sandbox/hidden"));
1424+
check!(symlink_dir("hidden/", &tmpdir, "sandbox/indirect"));
1425+
1426+
let sandbox = check!(tmpdir.open_dir("sandbox"));
1427+
1428+
for path in ["hidden", "hidden/", "indirect", "indirect/"] {
1429+
error!(
1430+
sandbox.open_dir(path),
1431+
"a path led outside of the filesystem"
1432+
);
1433+
error!(
1434+
sandbox.read_dir(path),
1435+
"a path led outside of the filesystem"
1436+
);
1437+
error!(
1438+
sandbox.canonicalize(path),
1439+
"a path led outside of the filesystem"
1440+
);
1441+
}
1442+
}
1443+
1444+
/// Similar to `trailing_slash_symlink`, but populates the test directory
1445+
/// outside the sandbox, so it can cover more cases.
1446+
#[test]
1447+
fn trailing_slash_symlink_more() {
1448+
let tmpdir = tempfile::tempdir().unwrap();
1449+
1450+
check!(std::fs::create_dir(tmpdir.path().join("sandbox")));
1451+
#[cfg(unix)]
1452+
{
1453+
check!(std::os::unix::fs::symlink(
1454+
"../outside",
1455+
tmpdir.path().join("sandbox/hidden")
1456+
));
1457+
check!(std::os::unix::fs::symlink(
1458+
"hidden/",
1459+
tmpdir.path().join("sandbox/indirect")
1460+
));
1461+
check!(std::os::unix::fs::symlink(
1462+
"/.",
1463+
tmpdir.path().join("sandbox/root_link")
1464+
));
1465+
}
1466+
#[cfg(windows)]
1467+
{
1468+
check!(std::os::windows::fs::symlink_dir(
1469+
"../outside",
1470+
tmpdir.path().join("sandbox/hidden")
1471+
));
1472+
check!(std::os::windows::fs::symlink_dir(
1473+
"hidden/",
1474+
tmpdir.path().join("sandbox/indirect")
1475+
));
1476+
check!(std::os::windows::fs::symlink_dir(
1477+
"/.",
1478+
tmpdir.path().join("sandbox/root_link")
1479+
));
1480+
}
1481+
#[cfg(not(any(unix, windows)))]
1482+
{
1483+
compile_error!("not implemented yet");
1484+
}
1485+
1486+
let tmpdir = check!(Dir::open_ambient_dir(tmpdir.path(), ambient_authority()));
1487+
1488+
let sandbox = check!(tmpdir.open_dir("sandbox"));
1489+
1490+
for path in [
1491+
"hidden",
1492+
"hidden/",
1493+
"indirect",
1494+
"indirect/",
1495+
"root_link",
1496+
"root_link/",
1497+
] {
1498+
error!(
1499+
sandbox.open_dir(path),
1500+
"a path led outside of the filesystem"
1501+
);
1502+
error!(
1503+
sandbox.read_dir(path),
1504+
"a path led outside of the filesystem"
1505+
);
1506+
error!(
1507+
sandbox.canonicalize(path),
1508+
"a path led outside of the filesystem"
1509+
);
1510+
}
1511+
}

0 commit comments

Comments
 (0)