Skip to content

Commit 232a8f5

Browse files
committed
tests: assert no arithmetic-input validator accepts a metacharacter
check-results and ksymoff both feed parsed fields into shell arithmetic, where $(( x )) evaluates embedded command substitutions and array subscripts -- and check-results is documented as running under sudo, so a value like a[$(cmd)] reaching it is root command execution. Four functions guard that across the two scripts: numeric() and hex16() in one, is_hex() and is_dec() in the other. They are duplicated because neither script can source a library: ksymoff installs to $PREFIX/bin as a standalone program, and check-results is copied to a target and run there. So a correction to one does not reach the others, and the failure mode of getting it wrong is root exec. The guard asserts REJECTION, not sameness. The four accept different sets deliberately -- is_hex refuses an 0x prefix because its callers strip one first, is_dec is decimal-only for --page-shift's range check -- so diffing them would fail on correct code. Every one must refuse a shared corpus of command substitutions, backticks, array subscripts and arithmetic operators. Two assertions beyond that. Each validator must ACCEPT a known-good value, so one that rejected everything could not pass the corpus vacuously while breaking the tool it guards. And the @arith-validator marker count must match the number exercised, so a validator added later cannot quietly escape the corpus -- the marker sits at the definition, making registration the same act as writing it. Definitions are extracted rather than sourced, both scripts running top-level code. Extraction also fails if a validator stops being self-contained, which is when a shared helper has crept in. Each definition now names the other three and records why they cannot be unified.
1 parent 2e223e9 commit 232a8f5

5 files changed

Lines changed: 201 additions & 1 deletion

File tree

Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,7 @@ lint :
686686
@$(TEST_DIR)/check-version
687687
@$(TEST_DIR)/check-posture-diff
688688
@$(TEST_DIR)/check-posture-summary
689+
@$(TEST_DIR)/check-validators
689690
@$(TEST_DIR)/check-shellcheck
690691
@$(TEST_DIR)/check-baseline
691692
@$(TEST_DIR)/check-render-parity

docs/testing.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ and `make` halts on the first.
142142
| `check-shellcheck` | shellcheck over the `extra/` helper scripts |
143143
| `check-confidence-floor` | no engine rule pins the *guaranteed* window from a guess — a sub-floor signal may shape `likely` only, outside the reviewed allowlist |
144144
| `check-unattributed-leak-floor` | a component that cannot establish *which* region its leaked value belongs to stays below the sound floor. Some leaks hand over a kernel address with no evidence of what it points at: the value is real, the region is a guess, and a region guess admitted to the guaranteed window is a soundness bug rather than an imprecision — an interior-text sample implies `image_base <= sample`, so a value that is not text but sits below the real `_text` carves the truth out of the window that promises to contain it. `/proc/<pid>/syscall` (CVE-2020-28588) is the registered case: it leaks whatever the reading task's call chain left on `proc_pid_syscall()`'s kernel stack frame, which is a return address into text on x86_32, arm and riscv32, and a direct-map pointer on powerpc and mips, where the image is randomized above them so the leaked word sits *below* `_text`. Nothing in the component can tell the two apart, because the text band is the unknown being solved for. The rule is per file rather than per call — a listed component has no region-establishing evidence at all, so no emission it makes can earn the sound band — which also keeps the matcher line-oriented, since a confidence argument routinely wraps onto its own line. A component that gains real corroboration is removed from the list, never raised in place |
145+
| `check-validators` | no arithmetic-input validator accepts anything dangerous. `extra/check-results` and `extra/ksymoff` both feed parsed fields into shell arithmetic, where `$(( x ))` evaluates embedded command substitutions — and `check-results` is documented as running under `sudo`, so a value like `a[$(cmd)]` reaching it would be root command execution. The validator is duplicated four ways because neither script can source a library (`ksymoff` installs to `$PREFIX/bin`; `check-results` is copied to a target), so a correction to one does not reach the others. What is asserted is *rejection*, not sameness: the four accept different sets on purpose. Also asserts each one accepts a known-good value, so a validator that rejected everything could not pass vacuously, and that the `@arith-validator` marker count matches the number exercised, so a new one cannot escape the corpus |
145146
| `check-arch-macros` | every macro an architecture header defines is read by something. A name nothing reads is a misspelling, a retired spelling one header kept, or dead weight — and the first two are silent: the architecture falls back to the contract's default for the macro it *meant* to set, which costs precision with nothing to show for it. No test catches that, because the tests read the same declaration the code does and assert whatever it says. Complements the retired-spelling `#error`s in `api.h`, which fail the build for one known-old name; this catches the names no such check lists |
146147
| `check-lattice-seam` | the quantities held to the estimate accessors (`Q_PAGE_OFFSET`, `Q_VA_BITS`) are read through `quantity_pinned/window/admits/narrowed`, never through `.lo` / `.hi`. `struct estimate` means different things per lattice — on a finite set `lo` is a live-candidate bitmask and `hi` is unused — and which lattice a quantity uses is declared once in the quantity table, so a direct read hard-codes an answer the reader never asked for. Nothing would fail loudly: a bitmask read as an address is a small integer, so the result is a plausible wrong answer rather than a crash. The pointer alias is discovered from its binding rather than assumed to be named `po`, so renaming it cannot slip a read past |
147148
| `check-page-offset-substitution` | no engine rule or leak component substitutes the compile-time `PAGE_OFFSET` for the target's linear-map base. That constant describes the analysing build, not the kernel under examination, and on the VMSPLIT arches the two differ routinely — code that reaches for it is asserting the split it was compiled with. The failure is invisible: it compiles everywhere, passes on the whole default-split corpus, and is off by exactly the gap between two build configurations, which is zero on every machine anyone tests. In a rule, an equality must read the resolved `Q_PAGE_OFFSET` via `quantity_pinned()`, and a bound may instead use `PAGE_OFFSET_MAX` (upper) or `PAGE_OFFSET_MIN` (lower), which hold against every target and need no resolution. A component runs before inference and can never see an estimate, so it measures the boundary instead — `kasld_kernel_pointer_floor()` for the user/kernel split, `kasld_page_offset_floor()` for a region-tagged bound. Comments and string literals are stripped first, and `#if` / `#elif` lines are exempt by construction (a constant expression cannot call an accessor, which is why the band assertions keep `PAGE_OFFSET` a plain scalar), so only C code counts |

extra/check-results

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,14 @@ skip() { update_counts skip; printf " ${YELLOW}SKIP${RESET} %s\n" "$1"; }
7272
# -------------------------------------------------------------------------
7373

7474
# Normalize a hex string (with or without 0x prefix) to 16-char lowercase
75+
#
76+
# @arith-validator — one of four guarding the same property across two scripts:
77+
# numeric() and hex16() here, is_hex() and is_dec() in extra/ksymoff. They are
78+
# duplicated because neither script can source a library (ksymoff installs to
79+
# $PREFIX/bin; this one is copied to a target), so a correction to one does NOT
80+
# reach the others -- review all four together. They accept different sets on
81+
# purpose; what none of them may do is accept a shell metacharacter, which
82+
# tests/check-validators asserts against a shared adversarial corpus.
7583
hex16() {
7684
# SECURITY: reject non-hex input up front so the reformatted value can never
7785
# carry shell/arithmetic metacharacters into a later `$(( 0x... ))` — the same
@@ -115,6 +123,8 @@ get_field() {
115123
# run `cmd` as root. So EVERY value that reaches `$(( ))` must pass through
116124
# numeric() first; anything with a non-[0-9a-fx] character is rejected to empty
117125
# (treated as an absent field by the callers' existing `[ -n ]` checks).
126+
# @arith-validator — see the note on hex16() above; all four are reviewed
127+
# together.
118128
numeric() {
119129
printf '%s' "$1" | grep -qiE '^(0x)?[0-9a-f]+$' && printf '%s' "$1"
120130
return 0 # never fail the substitution (a rejected value is just empty output)

extra/ksymoff

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,17 @@ hex_ult() { # a b (hex, no 0x) -> true if a < b unsigned
143143
# bash arithmetic evaluates embedded command substitutions and array subscripts,
144144
# so a value like `a[$(cmd)]` from an untrusted symbol source would execute as a
145145
# command. Every arithmetic input is validated by one of these first. (The same
146-
# guard is duplicated in extra/check-results — deliberately no shared library.)
146+
# guard is duplicated in extra/check-results — deliberately no shared library:
147+
# ksymoff installs to $PREFIX/bin and check-results is copied to a target, so
148+
# neither can source one. A correction here does NOT reach numeric()/hex16()
149+
# there -- review all four together. The accept sets differ on purpose (is_hex
150+
# refuses an 0x prefix, its callers having stripped one; is_dec is decimal-only
151+
# for --page-shift); what none may do is accept a shell metacharacter, which
152+
# tests/check-validators asserts against a shared adversarial corpus.
153+
#
154+
# @arith-validator
147155
is_hex() { case "$1" in ''|*[!0-9A-Fa-f]*) return 1 ;; *) return 0 ;; esac; }
156+
# @arith-validator
148157
is_dec() { case "$1" in ''|*[!0-9]*) return 1 ;; *) return 0 ;; esac; }
149158

150159
# -- Argument parsing -----------------------------------------------------

tests/check-validators

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
#!/bin/sh
2+
# This file is part of KASLD - https://github.com/bcoles/kasld
3+
#
4+
# check-validators — no arithmetic-input validator accepts anything dangerous.
5+
#
6+
# extra/check-results and extra/ksymoff both feed parsed fields into shell
7+
# arithmetic, and `$(( x ))` evaluates embedded command substitutions and array
8+
# subscripts. A value like `a[$(cmd)]` reaching it runs cmd -- as ROOT in
9+
# check-results, whose documented use is `sudo check-results results.txt` and
10+
# `<component> | sudo check-results -`. Each script therefore validates every
11+
# arithmetic input first.
12+
#
13+
# The guard exists because that validator is DUPLICATED. Neither script can
14+
# source a shared library: ksymoff is installed to $PREFIX/bin as a standalone
15+
# program, and check-results is meant to be copied to a target and run there. So
16+
# there are four implementations of one security property, and a correction to
17+
# one does not reach the others.
18+
#
19+
# What is asserted is REJECTION, not sameness. The four legitimately differ in
20+
# what they accept -- is_hex refuses an 0x prefix because its callers strip one
21+
# first, and is_dec is decimal-only because --page-shift feeds a decimal range
22+
# check. Diffing them would fail on correct code. The invariant that actually
23+
# matters is that none of them lets a shell metacharacter through.
24+
#
25+
# Also asserted:
26+
# - each validator ACCEPTS a known-good value, so one that rejected
27+
# everything could not pass the corpus vacuously while breaking the tool;
28+
# - the number of @arith-validator markers in the two scripts equals the
29+
# number exercised here, so a validator added later cannot quietly escape
30+
# the corpus.
31+
#
32+
# The definitions are extracted rather than sourced: both scripts run top-level
33+
# code and cannot be sourced. Extraction also fails loudly if a validator stops
34+
# being self-contained -- which is exactly when a shared helper has crept in.
35+
# ---
36+
# <bcoles@gmail.com>
37+
38+
set -u
39+
ROOT=$(CDPATH='' cd -- "$(dirname -- "$0")/.." && pwd)
40+
41+
if [ -t 1 ]; then
42+
RED=$(printf '\033[31m')
43+
GREEN=$(printf '\033[32m')
44+
RESET=$(printf '\033[0m')
45+
else
46+
RED=
47+
GREEN=
48+
RESET=
49+
fi
50+
51+
fail=0
52+
note() {
53+
printf '%scheck-validators: FAIL%s — %s\n' "$RED" "$RESET" "$1" >&2
54+
fail=1
55+
}
56+
57+
# Pull one shell function definition out of a script, by brace depth so a
58+
# one-liner and a multi-line body both come out whole.
59+
extract() {
60+
awk -v fn="$2" '
61+
index($0, fn "()") == 1 { inb = 1 }
62+
inb {
63+
print
64+
t = $0; o = gsub(/\{/, "{", t)
65+
t = $0; c = gsub(/\}/, "}", t)
66+
depth += o - c
67+
if (o > 0) seen = 1
68+
if (seen && depth <= 0) exit
69+
}
70+
' "$1"
71+
}
72+
73+
# Inputs no validator may ever accept. Anything here reaching `$(( ))` is
74+
# command execution, or arithmetic the caller never intended.
75+
corpus=$(
76+
cat <<'ADVERSARIAL'
77+
a[$(touch /tmp/kasld-pwn)]
78+
$(id)
79+
`id`
80+
$((1))
81+
${IFS}
82+
0x1$(id)
83+
x[0]
84+
1+1
85+
1-1
86+
1;id
87+
1 2
88+
-1
89+
+1
90+
0xg
91+
zz
92+
1.0
93+
0x
94+
*
95+
?
96+
../etc/passwd
97+
ADVERSARIAL
98+
)
99+
100+
# validator | script | convention | a value it MUST accept
101+
# echo — accepted when it echoes something back (rejection is empty output)
102+
# exit — accepted when it returns 0
103+
checks="
104+
numeric|extra/check-results|echo|deadbeef
105+
hex16|extra/check-results|echo|deadbeef
106+
is_hex|extra/ksymoff|exit|deadbeef
107+
is_dec|extra/ksymoff|exit|1234
108+
"
109+
110+
n_checked=0
111+
tmp=$(mktemp) || exit 1
112+
trap 'rm -f "$tmp"' EXIT INT TERM
113+
114+
# Read into positional args so the counters below survive (a `while read` on the
115+
# far side of a pipe runs in a subshell).
116+
oldifs=$IFS
117+
IFS='
118+
'
119+
# shellcheck disable=SC2086 # deliberate split: one row per newline
120+
set -- $checks
121+
IFS=$oldifs
122+
123+
for row in "$@"; do
124+
[ -n "$row" ] || continue
125+
fn=$(printf '%s' "$row" | cut -d'|' -f1)
126+
script=$(printf '%s' "$row" | cut -d'|' -f2)
127+
conv=$(printf '%s' "$row" | cut -d'|' -f3)
128+
good=$(printf '%s' "$row" | cut -d'|' -f4)
129+
src="$ROOT/$script"
130+
131+
if [ ! -f "$src" ]; then
132+
note "$script is missing"
133+
continue
134+
fi
135+
def=$(extract "$src" "$fn")
136+
if [ -z "$def" ]; then
137+
note "$script: cannot extract $fn() -- is it still a self-contained function?"
138+
continue
139+
fi
140+
n_checked=$((n_checked + 1))
141+
142+
# Non-vacuity first: a validator that rejects everything would sail through
143+
# the corpus below while breaking the tool it guards.
144+
if [ "$conv" = echo ]; then
145+
got=$(printf '%s\n' "$def" "out=\$($fn '$good'); [ -n \"\$out\" ]" | sh 2>/dev/null && echo yes)
146+
else
147+
got=$(printf '%s\n' "$def" "$fn '$good'" | sh 2>/dev/null && echo yes)
148+
fi
149+
[ "${got:-}" = yes ] ||
150+
note "$script: $fn() rejects '$good', which it must accept"
151+
152+
# Every adversarial input must be refused.
153+
printf '%s\n' "$corpus" | while IFS= read -r bad; do
154+
if [ "$conv" = echo ]; then
155+
out=$(printf '%s\n' "$def" "$fn \"\$1\"" | sh -s -- "$bad" 2>/dev/null)
156+
[ -z "$out" ] || printf 'ACCEPTED %s\n' "$bad"
157+
else
158+
printf '%s\n' "$def" "$fn \"\$1\"" | sh -s -- "$bad" 2>/dev/null &&
159+
printf 'ACCEPTED %s\n' "$bad"
160+
fi
161+
done >"$tmp" 2>/dev/null
162+
while IFS= read -r line; do
163+
note "$script: $fn() accepted ${line#ACCEPTED }"
164+
done <"$tmp"
165+
done
166+
167+
# A validator added later must not escape the corpus. The marker sits at each
168+
# definition, so registering one is the same act as writing it.
169+
markers=$(cat "$ROOT/extra/check-results" "$ROOT/extra/ksymoff" |
170+
grep -c '@arith-validator')
171+
if [ "${markers:-0}" -ne "$n_checked" ]; then
172+
note "$markers @arith-validator markers but $n_checked exercised -- register the new one in this guard"
173+
fi
174+
175+
if [ "$fail" -ne 0 ]; then
176+
exit 1
177+
fi
178+
printf '%scheck-validators: OK%s (%s arithmetic-input validators, none accepts the adversarial corpus)\n' \
179+
"$GREEN" "$RESET" "$n_checked"

0 commit comments

Comments
 (0)