Skip to content

Commit

Permalink
tests: add race attack tests for resolve_partial
Browse files Browse the repository at this point in the history
These tests are mostly based on the filepath-securejoin tests, though
with a few additional tests added to check for tricky symlinks.

Unfortunately, because the rename exchange thread can affect any openat2
call, we cannot run these tests in parallel with other tests (otherwise
you get spurrious errors because one test gets -EAGAIN too many times).
So we split the rust test execution into two groups manually (nextest
doesn't support natively specifying a test group as being "exclusive" in
this way, unfortunately).

Signed-off-by: Aleksa Sarai <[email protected]>
  • Loading branch information
cyphar committed Jan 15, 2025
1 parent 8fd9b78 commit b40251f
Show file tree
Hide file tree
Showing 6 changed files with 987 additions and 2 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ clap = { version = "^3", features = ["cargo"] }
errno = "^0.3"
tempfile = "^3"
paste = "^1"
path-clean = "^1"
pretty_assertions = "^1"

[lints.rust]
Expand Down
7 changes: 6 additions & 1 deletion hack/rust-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,9 @@ function nextest_run() {

set -x

nextest_run --no-fail-fast
# We need to run race and non-race tests separately because the racing tests
# can cause the non-race tests to error out spuriously. Hopefully in the future
# <https://github.com/nextest-rs/nextest/discussions/2054> will be resolved and
# nextest will make it easier to do this.
nextest_run --no-fail-fast -E "not test(#tests::test_race_*)"
nextest_run --no-fail-fast -E "test(#tests::test_race_*)"
2 changes: 2 additions & 0 deletions src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,5 @@ mod test_procfs;
mod test_resolve;
mod test_resolve_partial;
mod test_root_ops;

mod test_race_resolve_partial;
37 changes: 36 additions & 1 deletion src/tests/common/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/

use std::{fs, os::unix::fs as unixfs, path::Path};
use std::{
fs,
os::unix::fs as unixfs,
path::{Path, PathBuf},
};

use crate::syscalls;

Expand Down Expand Up @@ -230,3 +234,34 @@ pub(crate) fn create_basic_tree() -> Result<TempDir, Error> {
"setgid-other" => #[cfg(feature = "_test_as_root")] (dir, {chown 12345:12345}, {chmod 0o7777});
})
}

pub(crate) fn create_race_tree() -> Result<(TempDir, PathBuf), Error> {
let tmpdir = create_tree! {
// Our root.
"root" => (dir);
// The path that the race tests will try to operate on.
"root/a/b/c/d" => (dir);
// Symlinks to swap that are semantically equivalent but should also
// trigger breakout errors.
"root/b-link" => (symlink -> "../b/../b/../b/../b/../b/../b/../b/../b/../b/../b/../b/../b/../b");
"root/c-link" => (symlink -> "../../b/c/../../b/c/../../b/c/../../b/c/../../b/c/../../b/c/../../b/c/../../b/c/../../b/c");
// Bad paths that should result in an error.
"root/bad-link1" => (symlink -> "/non/exist");
"root/bad-link2" => (symlink -> "/file/non/exist");
// Try to attack the root to get access to /etc/passwd.
"root/etc/passwd" => (file);
"root/etc-target/passwd" => (file);
"root/etc-attack-rel-link" => (symlink -> "../../../../../../../../../../../../../../../../../../etc");
"root/etc-attack-abs-link" => (symlink -> "/../../../../../../../../../../../../../../../../../../etc");
"root/passwd-attack-rel-link" => (symlink -> "../../../../../../../../../../../../../../../../../../etc/passwd");
"root/passwd-attack-abs-link" => (symlink -> "/../../../../../../../../../../../../../../../../../../etc/passwd");
// File to swap a directory with.
"root/file" => (file);
// Directory outside the root we can swap with.
"outsideroot" => (dir);
};

let root: PathBuf = [tmpdir.as_ref(), Path::new("root")].iter().collect();

Ok((tmpdir, root))
}
Loading

0 comments on commit b40251f

Please sign in to comment.