security: close filesystem race findings
This commit is contained in:
Cristian Cezar Moisés
parent
8cca841258
commit
d8668e6d64
10 changed files with 698 additions and 133 deletions
|
|
@ -3,6 +3,11 @@
|
|||
set -Eeuo pipefail
|
||||
|
||||
bin=${1:-./zupt}
|
||||
repo_root=$(CDPATH='' cd -- "$(dirname -- "$0")/.." && pwd -P)
|
||||
case $bin in
|
||||
/*) ;;
|
||||
*) bin="$(pwd -P)/${bin#./}" ;;
|
||||
esac
|
||||
tmp=$(mktemp -d "${TMPDIR:-/tmp}/zupt-bench-safety.XXXXXXXX")
|
||||
trap 'rm -rf -- "$tmp"' EXIT HUP INT TERM
|
||||
|
||||
|
|
@ -11,9 +16,36 @@ fail() {
|
|||
exit 1
|
||||
}
|
||||
|
||||
# CodeQL #7 reported the old lstat(child) -> recursive pathname operation as
|
||||
# cpp/toctou-race-condition. Keep the platform-specific cleanup primitives in
|
||||
# the source gate as well as exercising the runtime symlink boundary below.
|
||||
cleanup_source=$repo_root/src/zupt_main.c
|
||||
grep -Fq 'static int zupt_remove_temp_tree_fd(int directory_fd)' \
|
||||
"$cleanup_source" || fail 'POSIX descriptor-relative cleanup is missing'
|
||||
grep -Fq 'unlinkat(parent_fd, entry->d_name, 0)' "$cleanup_source" ||
|
||||
fail 'POSIX leaf cleanup is not unlinkat-relative'
|
||||
grep -Fq 'directory_handle, data.cFileName, 1, 0)' "$cleanup_source" ||
|
||||
fail 'Windows recursive cleanup is not handle-relative'
|
||||
grep -Fq 'FILE_OPEN_REPARSE_POINT' "$cleanup_source" ||
|
||||
fail 'Windows cleanup no longer opens reparse points without following'
|
||||
grep -Fq 'zupt_win_delete_cleanup_entry(' "$cleanup_source" ||
|
||||
fail 'Windows cleanup lacks identity-checked handle deletion'
|
||||
grep -Fq 'current.nFileIndexLow == expected->nFileIndexLow' "$cleanup_source" ||
|
||||
fail 'Windows cleanup no longer rejects a close/reopen name exchange'
|
||||
if grep -Fq 'RemoveDirectoryW(full)' "$cleanup_source"; then
|
||||
fail 'Windows root cleanup restored post-handle pathname deletion'
|
||||
fi
|
||||
if grep -Fq 'lstat(child' "$cleanup_source" ||
|
||||
grep -Fq 'zupt_remove_temp_tree(child' "$cleanup_source"; then
|
||||
fail 'temporary cleanup restored a check-then-use pathname traversal'
|
||||
fi
|
||||
|
||||
case $(uname -s 2>/dev/null || printf unknown) in
|
||||
MINGW*|MSYS*|CYGWIN*)
|
||||
printf 'SKIP: historical POSIX /tmp symlink benchmark test is not native on Windows\n'
|
||||
"$bin" bench --compare >/dev/null 2>&1 ||
|
||||
fail 'native Windows handle-relative benchmark cleanup failed'
|
||||
printf 'SKIP: adversarial POSIX symlink injection is not native on Windows\n'
|
||||
printf 'private Windows handle-relative benchmark workspace: PASS\n'
|
||||
exit 0
|
||||
;;
|
||||
esac
|
||||
|
|
@ -46,4 +78,50 @@ if [[ -d $old_directory ]]; then
|
|||
mv "$old_directory" "$tmp/historical-remnant"
|
||||
fi
|
||||
|
||||
printf 'private benchmark workspace: PASS\n'
|
||||
# Inject a directory symlink into the private workspace while a real benchmark
|
||||
# is active. Cleanup must remove the link itself and never visit its target.
|
||||
mkdir "$tmp/symlink-target"
|
||||
printf 'cleanup sentinel must survive\n' > "$tmp/symlink-target/sentinel"
|
||||
cp "$tmp/symlink-target/sentinel" "$tmp/symlink-target.expected"
|
||||
dd if=/dev/urandom of="$tmp/injection-input" bs=65536 count=128 2>/dev/null
|
||||
|
||||
physical_tmp=$(CDPATH='' cd -P -- /tmp && pwd -P)
|
||||
: > "$tmp/preexisting-workspaces"
|
||||
for candidate in "$physical_tmp"/zupt-bench-*; do
|
||||
if [[ -d $candidate && ! -L $candidate ]]; then
|
||||
printf '%s\n' "$candidate" >> "$tmp/preexisting-workspaces"
|
||||
fi
|
||||
done
|
||||
|
||||
(cd "$tmp" && "$bin" bench injection-input >/dev/null 2>&1) &
|
||||
bench_pid=$!
|
||||
injected=0
|
||||
injected_workspace=
|
||||
attempt=0
|
||||
while (( attempt < 1000 )); do
|
||||
for candidate in "$physical_tmp"/zupt-bench-*; do
|
||||
[[ -d $candidate && ! -L $candidate ]] || continue
|
||||
if grep -Fqx -- "$candidate" "$tmp/preexisting-workspaces"; then
|
||||
continue
|
||||
fi
|
||||
if ln -s "$tmp/symlink-target" "$candidate/attacker-link" \
|
||||
2>/dev/null; then
|
||||
injected=1
|
||||
injected_workspace=$candidate
|
||||
break
|
||||
fi
|
||||
done
|
||||
(( injected == 1 )) && break
|
||||
kill -0 "$bench_pid" 2>/dev/null || break
|
||||
sleep 0.01
|
||||
attempt=$((attempt + 1))
|
||||
done
|
||||
wait "$bench_pid" || fail 'benchmark with injected symlink failed'
|
||||
(( injected == 1 )) || fail 'could not observe the private benchmark workspace'
|
||||
if [[ -e $injected_workspace || -L $injected_workspace ]]; then
|
||||
fail 'injected workspace was not the benchmark tree that was removed'
|
||||
fi
|
||||
cmp "$tmp/symlink-target.expected" "$tmp/symlink-target/sentinel" ||
|
||||
fail 'temporary cleanup followed an injected directory symlink'
|
||||
|
||||
printf 'private descriptor/handle-relative benchmark workspace: PASS\n'
|
||||
|
|
|
|||
|
|
@ -56,9 +56,30 @@ if "$binary" test --pass-fd not-a-number archive.zupt >/dev/null 2>&1; then
|
|||
exit 1
|
||||
fi
|
||||
|
||||
if "$binary" test --password-prompt archive.zupt </dev/null >/dev/null 2>&1; then
|
||||
prompt_log=$test_root/non-interactive-prompt.log
|
||||
if command -v timeout >/dev/null 2>&1; then
|
||||
set +e
|
||||
timeout 10 "$binary" test --password-prompt archive.zupt \
|
||||
</dev/null >"$prompt_log" 2>&1
|
||||
prompt_status=$?
|
||||
set -e
|
||||
else
|
||||
set +e
|
||||
"$binary" test --password-prompt archive.zupt \
|
||||
</dev/null >"$prompt_log" 2>&1
|
||||
prompt_status=$?
|
||||
set -e
|
||||
fi
|
||||
if ((prompt_status == 124)); then
|
||||
printf '%s\n' 'FAIL: non-interactive password prompt timed out' >&2
|
||||
exit 1
|
||||
elif ((prompt_status == 0)); then
|
||||
printf '%s\n' 'FAIL: non-interactive password prompt unexpectedly succeeded' >&2
|
||||
exit 1
|
||||
elif ! grep -Fq 'password prompt requires a terminal.' "$prompt_log"; then
|
||||
printf 'FAIL: non-interactive password prompt returned status %d without a terminal rejection\n' \
|
||||
"$prompt_status" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
case $(uname -s) in
|
||||
|
|
|
|||
|
|
@ -154,16 +154,19 @@ case "$(uname -s)" in
|
|||
|
||||
tree=$(fresh_tree raw-c1-path)
|
||||
control_name=$'raw-\200.txt'
|
||||
printf '\177ELF\002\001\001\000compiled' >"$tree/$control_name"
|
||||
if "$SCANNER" --tree "$tree" >"$TEST_TMP/output" 2>&1; then
|
||||
printf 'not ok - raw C1 path was not rejected\n'
|
||||
exit 1
|
||||
elif ! grep -Fq 'raw-\x80.txt' "$TEST_TMP/output" ||
|
||||
LC_ALL=C grep -q $'\200' "$TEST_TMP/output"; then
|
||||
printf 'not ok - raw C1 path was not rendered safely\n'
|
||||
exit 1
|
||||
if { printf '\177ELF\002\001\001\000compiled' >"$tree/$control_name"; } 2>/dev/null; then
|
||||
if "$SCANNER" --tree "$tree" >"$TEST_TMP/output" 2>&1; then
|
||||
printf 'not ok - raw C1 path was not rejected\n'
|
||||
exit 1
|
||||
elif ! grep -Fq 'raw-\x80.txt' "$TEST_TMP/output" ||
|
||||
LC_ALL=C grep -q $'\200' "$TEST_TMP/output"; then
|
||||
printf 'not ok - raw C1 path was not rendered safely\n'
|
||||
exit 1
|
||||
else
|
||||
pass 'scanner escapes invalid raw C1 bytes in reported paths'
|
||||
fi
|
||||
else
|
||||
pass 'scanner escapes invalid raw C1 bytes in reported paths'
|
||||
skip 'raw C1 filenames are forbidden by this filesystem'
|
||||
fi
|
||||
|
||||
tree=$(fresh_tree utf8-c1-path)
|
||||
|
|
|
|||
|
|
@ -187,6 +187,31 @@ else
|
|||
F "ECHO bit-clear missing the explicit (tcflag_t) cast"
|
||||
fi
|
||||
|
||||
# A restore to a device is irreversible. Classify the already-open descriptor
|
||||
# rather than checking target_path and resolving that mutable name again.
|
||||
if grep -Fq 'lstat(target_path' src/zupt_disk.c; then
|
||||
F "disk restore has a path-check/open TOCTOU pattern"
|
||||
elif grep -Fq 'tgt_fd = open(target_path' src/zupt_disk.c &&
|
||||
grep -Fq 'fstat(tgt_fd, &opened_st)' src/zupt_disk.c; then
|
||||
P "disk restore classifies the opened target descriptor"
|
||||
else
|
||||
F "disk restore descriptor-first target guard is missing"
|
||||
fi
|
||||
|
||||
# CodeQL #5 reported chmod(dst, mode) after reopening/resolving the SDK save
|
||||
# path. Key copies must use the core's handle/descriptor-relative atomic
|
||||
# publisher and apply POSIX permissions to its already-open temporary stream.
|
||||
if grep -Fq 'chmod(dst, mode)' sdk/src/zuptsdk.c; then
|
||||
F "SDK key save has a path-based chmod TOCTOU pattern"
|
||||
elif grep -Fq 'zupt_atomic_output_open(dst, &fo)' sdk/src/zuptsdk.c &&
|
||||
grep -Fq 'fchmod(fileno(fo), mode)' sdk/src/zuptsdk.c &&
|
||||
grep -Fq 'zupt_atomic_output_finish(output, rc == ZUPTSDK_OK)' \
|
||||
sdk/src/zuptsdk.c; then
|
||||
P "SDK key save uses descriptor-relative atomic publication"
|
||||
else
|
||||
F "SDK key save atomic publication guard is missing"
|
||||
fi
|
||||
|
||||
echo ""
|
||||
echo " ───────────────────────────────────────"
|
||||
echo " Static analysis: $PASS passed, $FAIL failed"
|
||||
|
|
|
|||
Loading…
Reference in a new issue