Skip to content

Commit bdeecaf

Browse files
committed
sandlock-core: confine O_EXCL existence probe and guard statx buffer length
Signed-off-by: Cong Wang <cwang@multikernel.io>
1 parent 5979107 commit bdeecaf

2 files changed

Lines changed: 33 additions & 8 deletions

File tree

crates/sandlock-core/src/cow/seccomp.rs

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -377,10 +377,11 @@ impl SeccompCowBranch {
377377

378378
// O_EXCL: fail if file already exists (in upper or lower)
379379
if flags & O_CREAT != 0 && flags & O_EXCL != 0 {
380-
let upper_file = self.upper.join(&rel);
381-
let lower_file = self.workdir.join(&rel);
382-
if upper_file.exists() || upper_file.is_symlink()
383-
|| lower_file.exists() || lower_file.is_symlink()
380+
// Confined existence check: a symlinked parent component must not
381+
// let this probe follow out of the tree, which would turn O_EXCL
382+
// into a host-file existence oracle (issue #112).
383+
if crate::sys::fs::statat_in_root(&self.upper, &rel, false).is_ok()
384+
|| crate::sys::fs::statat_in_root(&self.workdir, &rel, false).is_ok()
384385
{
385386
return Err(BranchError::Exists);
386387
}
@@ -441,10 +442,11 @@ impl SeccompCowBranch {
441442

442443
// O_EXCL: fail if file already exists
443444
if flags & O_CREAT != 0 && flags & O_EXCL != 0 {
444-
let upper_file = self.upper.join(&rel);
445-
let lower_file = self.workdir.join(&rel);
446-
if upper_file.exists() || upper_file.is_symlink()
447-
|| lower_file.exists() || lower_file.is_symlink()
445+
// Confined existence check: a symlinked parent component must not
446+
// let this probe follow out of the tree, which would turn O_EXCL
447+
// into a host-file existence oracle (issue #112).
448+
if crate::sys::fs::statat_in_root(&self.upper, &rel, false).is_ok()
449+
|| crate::sys::fs::statat_in_root(&self.workdir, &rel, false).is_ok()
448450
{
449451
return Err(BranchError::Exists);
450452
}
@@ -1583,4 +1585,22 @@ mod tests {
15831585
std::path::Path::new("existing.txt")
15841586
);
15851587
}
1588+
1589+
#[test]
1590+
fn o_excl_does_not_probe_through_symlinked_parent() {
1591+
// workdir/evil -> /etc ; open("evil/group", O_CREAT|O_EXCL) must not
1592+
// report EEXIST based on the host /etc/group: the existence probe is
1593+
// confined, so it cannot become a host-file oracle (issue #112).
1594+
let (workdir, storage) = setup_workdir();
1595+
std::os::unix::fs::symlink("/etc", workdir.path().join("evil")).unwrap();
1596+
let mut branch =
1597+
SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap();
1598+
let wd = workdir.path().canonicalize().unwrap();
1599+
let path = format!("{}/evil/group", wd.display());
1600+
let flags = (libc::O_CREAT | libc::O_EXCL | libc::O_WRONLY) as u64;
1601+
assert!(
1602+
!matches!(branch.prepare_open(&path, flags), Err(BranchError::Exists)),
1603+
"O_EXCL followed a symlinked parent into the host /etc/group"
1604+
);
1605+
}
15861606
}

crates/sandlock-core/src/sys/fs.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,11 @@ pub(crate) fn statx_in_root(
130130
mask: u32,
131131
buf: &mut [u8],
132132
) -> Result<(), i32> {
133+
// The kernel writes sizeof(struct statx) = 256 bytes; refuse a short buffer
134+
// rather than risk a heap overflow if a future caller passes a smaller one.
135+
if buf.len() < 256 {
136+
return Err(libc::EINVAL);
137+
}
133138
let follow = (flags & libc::AT_SYMLINK_NOFOLLOW) == 0;
134139
let fd = opath_in_root(root, path, follow)?;
135140
// statx the handle itself via AT_EMPTY_PATH; follow/nofollow is already

0 commit comments

Comments
 (0)