From 079d11651159d0e1850e45bb734be1f64b8328c2 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sat, 29 Aug 2026 16:01:01 +0200 Subject: [PATCH] Cover the Plymouth parent-chain walk and guard its test harness Deleting the whole ancestor walk from validate_trusted_directory and checking only the immediate parent left the suite green, so the invariant the design rests on had no coverage: a writable ancestor lets an attacker swap a validated directory out from under the leaf. Reject a run whose destination grandparent is world-writable while the destination itself is pristine. The harness also rewrites the root script's fixed /usr/share paths by string substitution. A drifted string silently no-ops that rewrite and would point the simulation at the real system tree, so assert each substitution landed. Assert the scratch directory exists too: a failed mktemp left it empty and the suite carried on building paths from it. Co-Authored-By: Claude Opus 5 (1M context) Co-Authored-By: Codex XHigh --- test/shell.d/plymouth-set-test.sh | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/shell.d/plymouth-set-test.sh b/test/shell.d/plymouth-set-test.sh index ae146c98..77d21af3 100755 --- a/test/shell.d/plymouth-set-test.sh +++ b/test/shell.d/plymouth-set-test.sh @@ -5,6 +5,8 @@ set -uo pipefail source "$(dirname "${BASH_SOURCE[0]}")/base-test.sh" test_tmp=$(mktemp -d) +[[ -n $test_tmp && -d $test_tmp ]] || + fail "the test creates its own scratch directory before touching anything" secret="$test_tmp/secret" trap 'chmod 0600 "$secret" 2>/dev/null || true; rm -rf -- "$test_tmp"' EXIT @@ -174,6 +176,13 @@ case "$1" in code=${code/PATH=\/usr\/bin:\/bin/PATH=$TEST_ROOT_TOOLS:\/usr\/bin:\/bin} code=${code/theme_dir=\/usr\/share\/plymouth\/themes\/omarchy/theme_dir=$TEST_FAKE_ROOT\/usr\/share\/plymouth\/themes\/omarchy} code=${code/sddm_dir=\/usr\/share\/sddm\/themes\/omarchy/sddm_dir=$TEST_FAKE_ROOT\/usr\/share\/sddm\/themes\/omarchy} + + # Each rewrite above silently no-ops if the production text drifts, which + # would point this simulation at the real /usr/share. Refuse instead. + [[ $code == *"PATH=$TEST_ROOT_TOOLS:/usr/bin:/bin"* ]] || exit 94 + [[ $code == *"theme_dir=$TEST_FAKE_ROOT/usr/share/plymouth/themes/omarchy"* ]] || exit 94 + [[ $code == *"sddm_dir=$TEST_FAKE_ROOT/usr/share/sddm/themes/omarchy"* ]] || exit 94 + PATH="$TEST_ROOT_TOOLS:/usr/bin:/bin" \ /bin/bash -c "$code" "$shell_name" "$@" ;; @@ -472,6 +481,24 @@ assert_no_temporary_files "$fake_root" pass "publication rejects symlinked and non-root-writable destination parents" +# Walking the whole chain, not just the immediate parent, is what closes the +# rename race: a writable ancestor lets an attacker swap an entire validated +# directory out from under the leaf. Leave the destination itself pristine so +# only the ancestor can be at fault. +setup_run +chmod 0777 "$fake_root/usr/share/plymouth" +output=$(run_set 022 env 2>&1) +status=$? +chmod 0755 "$fake_root/usr/share/plymouth" + +(( status != 0 )) || fail "a writable destination ancestor is rejected" "$output" +[[ $(stat -c %a "$theme") == 755 ]] || fail "only the ancestor, not the destination, was untrustworthy" +[[ $(cat "$theme/bullet.png") == 'old plymouth bullet.png' ]] || fail "a writable ancestor leaves the live destination unchanged" +[[ $output == *"refusing to publish"* ]] || fail "a rejected ancestor says why it refused" "$output" +assert_no_temporary_files "$fake_root" + +pass "publication walks the whole parent chain, not only the immediate parent" + # Refresh uses the same publisher but its explicit contract includes the # packaged nested logos/oma.png asset. It must not touch the SDDM theme. setup_run