tests: add race attack tests for resolve_partial

Cargo.toml

@@ -64,6 +64,7 @@ clap = { version = "^3", features = ["cargo"] }
 errno = "^0.3"
 tempfile = "^3"
 paste = "^1"
+path-clean = "^1"
 pretty_assertions = "^1"
 
 [lints.rust]

Makefile

@@ -66,17 +66,11 @@ test-rust-doctest:
 
 .PHONY: test-rust-unpriv
 test-rust-unpriv:
-	$(CARGO_NIGHTLY) llvm-cov --no-report --branch --features capi nextest --no-fail-fast
+	./hack/rust-tests.sh --cargo="$(CARGO_NIGHTLY)"
 
 .PHONY: test-rust-root
 test-rust-root:
-# Run Rust tests as root.
-# NOTE: Ideally this would be configured in .cargo/config.toml so it
-#       would also work locally, but unfortunately it seems cargo doesn't
-#       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
+	./hack/rust-tests.sh --cargo="$(CARGO_NIGHTLY)" --sudo
 
 .PHONY: test-rust
 test-rust:

hack/rust-tests.sh

@@ -0,0 +1,79 @@
+#!/bin/bash
+# libpathrs: safe path resolution on Linux
+# Copyright (C) 2019-2025 Aleksa Sarai <[email protected]>
+# Copyright (C) 2019-2025 SUSE LLC
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Lesser General Public License as published by the Free
+# Software Foundation, either version 3 of the License, or (at your option) any
+# later version.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT ANY
+# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+# PARTICULAR PURPOSE. See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public License along
+# with this program. If not, see <https://www.gnu.org/licenses/>.
+
+set -Eeuo pipefail
+
+TEMP="$(getopt -o sc: --long sudo,cargo: -- "$@")"
+eval set -- "$TEMP"
+
+function bail() {
+	echo "rust tests: $*" >&2
+	exit 1
+}
+
+sudo=
+CARGO="${CARGO_NIGHTLY:-cargo +nightly}"
+while [ "$#" -gt 0 ]; do
+	case "$1" in
+		-s|--sudo)
+			sudo=1
+			shift
+			;;
+		-c|--cargo)
+			CARGO="$2"
+			shift 2
+			;;
+		--)
+			shift
+			break
+			;;
+		*)
+			bail "unknown option $1"
+	esac
+done
+
+function nextest_run() {
+	features=("capi")
+
+	if [ -v CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER ]; then
+		unset CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER
+	fi
+
+	if [ -n "$sudo" ]; then
+		features+=("_test_as_root")
+
+		# This CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER magic lets us run Rust
+		# tests as root without needing to run the build step as root.
+		export CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="sudo -E"
+	fi
+
+	$CARGO llvm-cov --no-report --branch --features "$(printf "%s," "${features[@]}")" \
+		nextest "$@"
+
+	if [ -v CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER ]; then
+		unset CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER
+	fi
+}
+
+set -x
+
+# 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_*)"

src/error.rs

@@ -56,6 +56,10 @@ impl Error {
     pub fn kind(&self) -> ErrorKind {
         self.0.kind()
     }
+
+    pub(crate) fn is_safety_violation(&self) -> bool {
+        self.0.is_safety_violation()
+    }
 }
 
 #[derive(thiserror::Error, Debug)]
@@ -150,6 +154,10 @@ impl ErrorImpl {
             Self::Wrapped { source, .. } => source.kind(),
         }
     }
+
+    pub(crate) fn is_safety_violation(&self) -> bool {
+        self.kind().is_safety_violation()
+    }
 }
 
 impl ErrorKind {
@@ -158,7 +166,6 @@ impl ErrorKind {
     /// Aside from fetching the errno represented by standard
     /// [`ErrorKind::OsError`] errors, pure-Rust errors are also mapped to C
     /// errno values where appropriate.
-    #[cfg(any(feature = "capi", test))]
     pub(crate) fn errno(&self) -> Option<i32> {
         match self {
             ErrorKind::NotImplemented => Some(libc::ENOSYS),
@@ -168,6 +175,10 @@ impl ErrorKind {
             _ => None,
         }
     }
+
+    pub(crate) fn is_safety_violation(&self) -> bool {
+        self.errno() == Self::SafetyViolation.errno()
+    }
 }
 
 // Private trait necessary to work around the "orphan trait" restriction.

src/resolvers/opath/imp.rs

@@ -237,6 +237,18 @@ fn do_resolve<Fd: AsFd, P: AsRef<Path>>(
                 // lexically. If pop() fails, then we are at the root.
                 // should .
                 if !expected_path.pop() {
+                    // If we hit ".." due to the symlink we need to drop it from
+                    // the stack like we would if we walked into a real
+                    // component. Otherwise walking into ".." will result in a
+                    // broken symlink stack error.
+                    if let Some(ref mut stack) = symlink_stack {
+                        stack
+                            .pop_part(&part)
+                            .map_err(|err| ErrorImpl::BadSymlinkStackError {
+                                description: "walking into component".into(),
+                                source: err,
+                            })?;
+                    }
                     current = Rc::clone(&root);
                     continue;
                 }

src/resolvers/openat2.rs

@@ -137,9 +137,16 @@ pub(crate) fn resolve_partial<Fd: AsFd>(
 
     // TODO: We probably want to do a git-bisect-like binary-search here. For
     //       paths with a large number of components this could make a
-    //       significant difference, though in practice you'll only see.
-    //       really large paths this could make a significant difference.
+    //       significant difference, though in practice you'll only see fairly
+    //       short paths so the implementation complexity might not be worth it.
     for (path, remaining) in path.partial_ancestors() {
+        if last_error.is_safety_violation() {
+            // If we hit a safety violation, we return an error instead of a
+            // partial resolution to match the behaviour of the O_PATH
+            // resolver (and to avoid some possible weird bug in libpathrs
+            // being exploited to return some result to Root::mkdir_all).
+            return Err(last_error);
+        }
         match resolve(root, path, rflags, no_follow_trailing) {
             Ok(handle) => {
                 return Ok(PartialLookup::Partial {
@@ -152,20 +159,5 @@ pub(crate) fn resolve_partial<Fd: AsFd>(
         }
     }
 
-    // Fall back to returning (root, path) if there was no path found.
-    //
-    // TODO: In theory you should never hit this case because
-    //       partial_ancestors() always returns a "root" value. This should
-    //       probably be unreachable!()...
-    Ok(PartialLookup::Partial {
-        handle: root
-            .try_clone_to_owned()
-            .map(Handle::from_fd)
-            .map_err(|err| ErrorImpl::OsError {
-                operation: "clone root".into(),
-                source: err,
-            })?,
-        remaining: path.into(),
-        last_error,
-    })
+    unreachable!("partial_ancestors should include root path which must be resolvable");
 }

src/tests.rs

@@ -67,3 +67,5 @@ mod test_procfs;
 mod test_resolve;
 mod test_resolve_partial;
 mod test_root_ops;
+
+mod test_race_resolve_partial;

src/tests/common/handle.rs

@@ -20,6 +20,7 @@
 use crate::{
     error::ErrorKind,
     flags::OpenFlags,
+    resolvers::PartialLookup,
     tests::{common as tests_common, traits::HandleImpl},
     utils::FdExt,
 };
@@ -35,6 +36,44 @@ use rustix::{
 
 pub type LookupResult<'a> = (&'a str, libc::mode_t);
 
+impl<H, E> PartialLookup<H, E> {
+    pub(in crate::tests) fn as_inner_handle(&self) -> &H {
+        match self {
+            PartialLookup::Complete(handle) => handle,
+            PartialLookup::Partial { handle, .. } => handle,
+        }
+    }
+}
+
+impl<H, E> PartialEq for PartialLookup<H, E>
+where
+    H: PartialEq,
+    E: PartialEq,
+{
+    fn eq(&self, other: &Self) -> bool {
+        match (self, other) {
+            (Self::Complete(left), Self::Complete(right)) => left == right,
+            (
+                Self::Partial {
+                    handle: left_handle,
+                    remaining: left_remaining,
+                    last_error: left_last_error,
+                },
+                Self::Partial {
+                    handle: right_handle,
+                    remaining: right_remaining,
+                    last_error: right_last_error,
+                },
+            ) => {
+                left_handle == right_handle
+                    && left_remaining == right_remaining
+                    && left_last_error == right_last_error
+            }
+            _ => false,
+        }
+    }
+}
+
 pub(in crate::tests) fn check_oflags<Fd: AsFd>(fd: Fd, flags: OpenFlags) -> Result<(), Error> {
     let fd = fd.as_fd();
 

src/tests/common/root.rs

@@ -17,14 +17,20 @@
  * 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;
 
 use anyhow::{Context, Error};
 use rustix::fs::{self as rustix_fs, AtFlags, OFlags, CWD};
 use tempfile::TempDir;
 
+// TODO: Make these macros usable from outside this crate...
+
 macro_rules! create_inode {
     // "/foo/bar" @ chmod 0o755
     (@do $path:expr, chmod $mode:expr) => {
@@ -140,13 +146,13 @@ macro_rules! create_tree {
                     create_inode!(&path => $($inner)*);
                 }
             )*
-            Ok(root)
+            root
         }
     }
 }
 
-pub fn create_basic_tree() -> Result<TempDir, Error> {
-    create_tree! {
+pub(crate) fn create_basic_tree() -> Result<TempDir, Error> {
+    Ok(create_tree! {
         // Basic inodes.
         "a" => (dir);
         "b/c/d/e/f" => (dir);
@@ -156,6 +162,8 @@ pub fn create_basic_tree() -> Result<TempDir, Error> {
         "root-link1" => (symlink -> "/");
         "root-link2" => (symlink -> "/..");
         "root-link3" => (symlink -> "/../../../../..");
+        "escape-link1" => (symlink -> "../../../../../../../../../../target");
+        "escape-link2" => (symlink -> "/../../../../../../../../../../target");
         // Some "bad" inodes that non-privileged users can create.
         "b/fifo" => (fifo);
         "b/sock" => (sock);
@@ -224,5 +232,36 @@ pub fn create_basic_tree() -> Result<TempDir, Error> {
         // setgid has unique behaviour when interacting with mkdir_all.
         "setgid-self" => (dir, {chmod 0o7777});
         "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))
 }