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).

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
  • Loading branch information
cyphar committed Jan 10, 2025
1 parent 13ec21d commit 1257b82
Show file tree
Hide file tree
Showing 7 changed files with 992 additions and 3 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
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ test-rust-doctest:

.PHONY: test-rust-unpriv
test-rust-unpriv:
$(CARGO_NIGHTLY) llvm-cov --no-report --branch --features capi nextest --no-fail-fast
$(CARGO_NIGHTLY) llvm-cov --no-report --branch --features capi nextest --config-file nextest.toml

.PHONY: test-rust-root
test-rust-root:
Expand All @@ -76,7 +76,7 @@ test-rust-root:
# support cfg(feature=...) for target runner configs.
# See <https://github.com/rust-lang/cargo/issues/14306>.
CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER='sudo -E' \
$(CARGO_NIGHTLY) llvm-cov --no-report --branch --features capi,_test_as_root nextest --no-fail-fast
$(CARGO_NIGHTLY) llvm-cov --no-report --branch --features capi,_test_as_root nextest --config-file nextest.toml

.PHONY: test-rust
test-rust:
Expand Down
12 changes: 12 additions & 0 deletions nextest.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[profile.default]
fail-fast = false

# The race tests cause our regular tests with openat2 to spurriously fail
# because the rename exchange can cause unrelated openat2(2) calls to return
# -EAGAIN. This configuration magic makes it so that these tests cannot run
# with other tests, though it unfortunately makes their execution serial. See
# <https://github.com/nextest-rs/nextest/discussions/2054> for a proposal to
# make this better.
[[profile.default.overrides]]
filter = "test(tests::test_race_resolve_partial::)"
threads-required = "num-test-threads"
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 1257b82

Please sign in to comment.