diff --git a/crates/fs-mistrust/src/imp.rs b/crates/fs-mistrust/src/imp.rs index bc935a414..b02aed861 100644 --- a/crates/fs-mistrust/src/imp.rs +++ b/crates/fs-mistrust/src/imp.rs @@ -10,6 +10,18 @@ use crate::{ Error, Result, Type, }; +/// Definition for the "sticky bit", which on Unix means that the contents of +/// directory may not be renamed, deleted, or otherwise modified by a non-owner +/// of those contents, even if the user has write permissions on the directory. +/// +/// This is the usual behavior for /tmp: You can make your own directories in +/// /tmp, but you can't modify other people's. +/// +/// (We'd use libc's version of `S_ISVTX`, but they vacillate between u16 and +/// u32 depending what platform you're on.) +#[cfg(target_family = "unix")] +pub(crate) const STICKY_BIT: u32 = 0o1000; + impl<'a> super::Verifier<'a> { /// Return an iterator of all the security problems with `path`. /// @@ -108,12 +120,24 @@ impl<'a> super::Verifier<'a> { 0o077 } else { // If this is the target object and it may be readable, or if - // this is _any parent directory_, then we only forbid the + // this is _any parent directory_, then we typically forbid the // group-write and all-write bits. (Those are the bits that // would allow non-trusted users to change the object, or change // things around in a directory.) - 0o022 - // TODO: Handle sticky bit. + if meta.is_dir() + && meta.mode() & STICKY_BIT != 0 + && path_type == PathType::Intermediate + { + // This is an intermediate directory and this sticky bit is + // set. Thus, we don't care if it is world-writable or + // group-writable, since only the _owner_ of a file in this + // directory can move or rename it. + 0o000 + } else { + // It's not a sticky-bit intermediate directory; actually + // forbid 022. + 0o022 + } }; let bad_bits = meta.mode() & forbidden_bits; if bad_bits != 0 { diff --git a/crates/fs-mistrust/src/lib.rs b/crates/fs-mistrust/src/lib.rs index e9a58dc40..e36a733bf 100644 --- a/crates/fs-mistrust/src/lib.rs +++ b/crates/fs-mistrust/src/lib.rs @@ -498,6 +498,33 @@ mod test { assert!(matches!(&errs[1], Error::NotFound(_))); } + #[cfg(target_family = "unix")] + #[test] + fn sticky() { + let d = Dir::new(); + d.dir("a/b/c"); + d.chmod("a", 0o777); + d.chmod("a/b", 0o755); + d.chmod("a/b/c", 0o700); + + let mut m = Mistrust::new(); + m.ignore_prefix(d.canonical_root()).unwrap(); + + // `a` is world-writable, so the first check will fail. + m.check_directory(d.path("a/b/c")).unwrap_err(); + + // Now `a` is world-writable _and_ sticky, so the check should succeed. + d.chmod("a", 0o777 | crate::imp::STICKY_BIT); + + m.check_directory(d.path("a/b/c")).unwrap(); + + // Make sure we got the right definition! + #[allow(clippy::useless_conversion)] + { + assert_eq!(crate::imp::STICKY_BIT, u32::from(libc::S_ISVTX)); + } + } + // TODO: Write far more tests. // * Can there be a test for a failed readlink()? I can't see an easy way // to provoke that without trying to make a time-of-check/time-of-use race