Skip to content

Commit a734207

Browse files
committed
api: fail a fact read that does not fit under the sysroot
kasld_resolve composes "<KASLD_SYSROOT><path>" into a KASLD_PATH_MAX buffer and returned the bare path where the two did not fit, so a long enough sysroot sent fact reads to the machine running the analysis while the output still presented them as a captured tree. A 4091-byte sysroot naming nothing read 124 facts; a short one naming nothing read 3. The comment argued that a miss against the live tree beat a truncated path, which weighs two options and omits the third. Nothing was salvageable in any case: the buffer is KASLD_PATH_MAX at all five call sites, so a prefixed path that does not fit is already longer than one the kernel will open, and the fallback could only ever substitute a live read for a read that had to fail. Return NULL and fail with ENAMETOOLONG, which is what happened. The five wrappers check it; kasld_read_file_line already propagates a null FILE, and nothing outside the header calls kasld_resolve, so those five are all of them. Short fact paths still resolve under an over-long root while long ones fail, which is a decline and widens the window rather than narrowing it. check-sysroot-containment compares two sysroots that both name nothing, one sized past the buffer, and requires them to read the same facts. A live run supplies the control: a host exposing no more than the empty sysroot does leaves nothing to detect, so the guard skips rather than passing on an absence.
1 parent 30034bc commit a734207

5 files changed

Lines changed: 154 additions & 9 deletions

File tree

Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -821,6 +821,7 @@ lint :
821821
$(TEST_DIR)/check-render-parity \
822822
$(TEST_DIR)/check-render-color \
823823
$(TEST_DIR)/check-wire-text \
824+
$(TEST_DIR)/check-sysroot-containment \
824825
$(TEST_DIR)/hardening-fixtures \
825826
$(TEST_DIR)/cli-flags
826827

SECURITY.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,21 @@ with a pointer to where it belongs rather than an advisory.
1414

1515
KASLD runs each component as a separate program and parses its output in the
1616
orchestrator, which is usually the more privileged process in a run. That
17-
asymmetry is where the interesting surface is:
17+
asymmetry, and the untrusted input a run consumes, are where the interesting
18+
surface is:
1819

1920
- **Orchestrator parsing.** `src/orchestrator.c` consumes component stdout, ELF
2021
section payloads and `dmesg` — input an unprivileged process can influence —
2122
inside the privileged process. Memory-safety, over-read or unbounded-loop
2223
bugs there are a genuine exposure surface; `tests/fuzz` covers these parsers,
2324
so a crashing input is directly actionable.
25+
- **Captured trees.** `KASLD_SYSROOT` redirects every kernel-fact path a run
26+
reads to a copy of another system's `/proc`, `/sys` and `/boot`, and
27+
`extra/collect` produces such trees to be analysed elsewhere. A tree is
28+
untrusted input — it comes from the machine under investigation, and its
29+
directory names and extraction path are as much a part of it as its file
30+
contents. Analysing one must not execute code from it, read outside it, or
31+
fall back to the analysing host's own state.
2432
- **The `extra/` helpers.** Several are documented as being run under `sudo`,
2533
so anything in one of them that mishandles untrusted input does so as root.
2634
- **Unintended execution or privilege change.** `KASLD_COMPONENT_DIR` and

docs/testing.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ stays plain, and setting `KASLD_COLOR` non-empty or empty forces either.
209209
| `check-render-parity` | the text readout, the markdown report and JSON name the same set of resolved quantities for a given run † |
210210
| `check-render-color` | coloured output is byte-identical to plain output once the escape sequences are removed, and markdown, JSON and oneline carry no escapes at all † |
211211
| `check-wire-text` | a component cannot put an escape sequence on the terminal: a record whose `name`, or a disposition whose `gate` or `msg`, leaves printable ASCII is rejected, and the verbose echo of component output strips control bytes † |
212+
| `check-sysroot-containment` | a `KASLD_SYSROOT` too long to build a fact path with fails the read instead of falling back to the analysing host's own `/proc` and `/sys`|
212213
| `check-guard-docs` | this table lists exactly the guards `make lint` runs — the same parity check `check-manpages` applies to flags, applied to the guard list itself |
213214
| `check-matrix-summary` | the summary table in `docs/reproducibility.md` restates the full per-scenario matrix it precedes: same cells, same KASLR state, same `default` and `perf-open` results in both directions |
214215
| `check-readout-docs` | documented sample output uses the renderer's current vocabulary and fits 100 columns (live output is measured separately by `check-render-width`) † |
@@ -510,6 +511,35 @@ differential against a colourless run passes while proving nothing. Determinism
510511
comes from an empty sysroot plus stub components, which also supply the pinned
511512
base the coloured branches need.
512513

514+
**`check-wire-text`** — A component's free text is data, and the fields
515+
carrying it — a result's `name`, a disposition's `gate` and `msg` — are
516+
rendered into the report an operator forwards. An erase-line sequence among
517+
them redraws a line already printed, so a finding can be made to read as its
518+
opposite by the report meant to expose it. `check-render-color` proves KASLD's
519+
own escapes strip back to the plain rendering, which says nothing about escapes
520+
arriving in data.
521+
522+
The admissible set stops at 0x7E rather than merely above 0x1F, because
523+
0x80..0x9F is the C1 control range and a terminal in an 8-bit locale acts on it
524+
with no ESC byte involved. Both halves are exercised. The guard also reads its
525+
own output with `grep -a`: without it a high byte makes grep report a binary
526+
match instead of lines, leaving the check searching nothing and passing against
527+
the very build it targets.
528+
529+
**`check-sysroot-containment`**`kasld_resolve()` composes
530+
`<KASLD_SYSROOT><path>` into a `KASLD_PATH_MAX` buffer, and returning the bare
531+
path where the two do not fit sends the read to the analysing machine's own
532+
`/proc` and `/sys` while the output still presents a captured tree. It is not a
533+
truncation trade-off: a prefixed path overflowing 4096 bytes is already longer
534+
than one the kernel will open, so the fallback never salvaged a read that would
535+
otherwise have worked. Before the fix, a 4091-byte sysroot naming nothing read
536+
124 facts where a short one naming nothing read 3.
537+
538+
Both roots name nothing, so a difference between them can only be a read that
539+
escaped. A live run supplies the control: a host exposing no more facts than
540+
the empty sysroot does leaves nothing to detect, and the guard skips rather
541+
than passing on an absence.
542+
513543
**`check-doc-structure`** — Three failures markdown accepts silently and a
514544
reader meets as a broken page: an unclosed fence swallows the rest of the
515545
document, a heading added without its TOC line is unreachable from the contents

src/include/kasld/sysroot.h

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#define KASLD_SYSROOT_H
2828

2929
#include <dirent.h>
30+
#include <errno.h>
3031
#include <fcntl.h>
3132
#include <stdio.h>
3233
#include <stdlib.h>
@@ -90,8 +91,13 @@ __attribute__((unused)) static void kasld_hermetic_record(const char *abs) {
9091

9192
/* Resolve an absolute fact path against the sysroot. With no sysroot set (or
9293
* a non-absolute path), returns `abs` unchanged. Otherwise writes
93-
* "<root><abs>" into buf and returns it; if that would not fit, falls back to
94-
* `abs` (a miss against the live tree is safer than a truncated path). */
94+
* "<root><abs>" into buf and returns it.
95+
*
96+
* Returns NULL if the prefixed path does not fit. buf is KASLD_PATH_MAX at
97+
* every call site, so a path that does not fit is already longer than one the
98+
* kernel will open: there is no reading it either way. Callers must check, and
99+
* fail the read with ENAMETOOLONG. Returning `abs` instead would read the
100+
* analysing machine's own file while a sysroot named another tree. */
95101
__attribute__((unused)) static const char *
96102
kasld_resolve(const char *abs, char *buf, size_t bufsz) {
97103
const char *root = kasld_sysroot();
@@ -105,7 +111,7 @@ kasld_resolve(const char *abs, char *buf, size_t bufsz) {
105111
rl = strlen(root);
106112
al = strlen(abs);
107113
if (rl + al + 1 > bufsz)
108-
return abs;
114+
return NULL;
109115
memcpy(buf, root, rl);
110116
memcpy(buf + rl, abs, al + 1);
111117
return buf;
@@ -114,33 +120,58 @@ kasld_resolve(const char *abs, char *buf, size_t bufsz) {
114120
__attribute__((unused)) static FILE *kasld_fopen(const char *path,
115121
const char *mode) {
116122
char buf[KASLD_PATH_MAX];
117-
return fopen(kasld_resolve(path, buf, sizeof(buf)), mode);
123+
const char *p = kasld_resolve(path, buf, sizeof(buf));
124+
if (!p) {
125+
errno = ENAMETOOLONG;
126+
return NULL;
127+
}
128+
return fopen(p, mode);
118129
}
119130

120131
/* Read-only open() only — kasld never creates files, so no mode arg (and
121132
* thus no variadic wrapper) is needed. */
122133
__attribute__((unused)) static int kasld_open(const char *path, int flags) {
123134
char buf[KASLD_PATH_MAX];
124-
return open(kasld_resolve(path, buf, sizeof(buf)), flags);
135+
const char *p = kasld_resolve(path, buf, sizeof(buf));
136+
if (!p) {
137+
errno = ENAMETOOLONG;
138+
return -1;
139+
}
140+
return open(p, flags);
125141
}
126142

127143
__attribute__((unused)) static int kasld_stat(const char *path,
128144
struct stat *st) {
129145
char buf[KASLD_PATH_MAX];
130-
return stat(kasld_resolve(path, buf, sizeof(buf)), st);
146+
const char *p = kasld_resolve(path, buf, sizeof(buf));
147+
if (!p) {
148+
errno = ENAMETOOLONG;
149+
return -1;
150+
}
151+
return stat(p, st);
131152
}
132153

133154
__attribute__((unused)) static int kasld_access(const char *path, int mode) {
134155
char buf[KASLD_PATH_MAX];
135-
return access(kasld_resolve(path, buf, sizeof(buf)), mode);
156+
const char *p = kasld_resolve(path, buf, sizeof(buf));
157+
if (!p) {
158+
errno = ENAMETOOLONG;
159+
return -1;
160+
}
161+
return access(p, mode);
136162
}
137163

138164
/* opendir() through the sysroot. Entry names are returned as-is (relative to
139165
* the dir), so callers rebuild child paths with the original (un-rewritten)
140166
* directory prefix and re-resolve via the other wrappers. */
141167
__attribute__((unused)) static DIR *kasld_opendir(const char *path) {
142168
char buf[KASLD_PATH_MAX];
143-
return opendir(kasld_resolve(path, buf, sizeof(buf)));
169+
const char *p = kasld_resolve(path, buf, sizeof(buf));
170+
if (!p) {
171+
errno = ENAMETOOLONG;
172+
return NULL;
173+
}
174+
return opendir(p);
144175
}
145176

146177
/* Read the first line of a fact file into buf: NUL-terminated, trailing

tests/check-sysroot-containment

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
#!/bin/sh
2+
# This file is part of KASLD - https://github.com/bcoles/kasld
3+
#
4+
# check-sysroot-containment — a sysroot too long to build a path with must not
5+
# send the read back to the analysing machine.
6+
#
7+
# kasld_resolve() writes "<KASLD_SYSROOT><path>" into a KASLD_PATH_MAX buffer.
8+
# Where the two do not fit, returning the bare path reads the analysing host's
9+
# own /proc and /sys while the output presents the result as a captured tree.
10+
# Nothing is salvageable at that point — the prefixed path is already longer
11+
# than one the kernel will open — so the read fails with ENAMETOOLONG.
12+
#
13+
# Two sysroots that name nothing are compared: one short, one long enough to
14+
# overflow. Both must yield the same facts. Whether the run reads anything is
15+
# not assumed: a live run supplies the control, and a host exposing no more
16+
# than the empty sysroot does leaves nothing to detect, so the guard skips
17+
# rather than passing on an absence.
18+
# ---
19+
# <bcoles@gmail.com>
20+
21+
set -u
22+
ROOT=$(CDPATH='' cd -- "$(dirname -- "$0")/.." && pwd)
23+
24+
if [ -t 1 ] || [ -n "${KASLD_COLOR:-}" ]; then
25+
RED=$(printf '\033[31m'); GREEN=$(printf '\033[32m'); YELLOW=$(printf '\033[33m')
26+
RESET=$(printf '\033[0m')
27+
else
28+
RED=; GREEN=; YELLOW=; RESET=
29+
fi
30+
31+
HOST_TRIPLE=$(cc -dumpmachine 2>/dev/null || echo unknown)
32+
BIN="$ROOT/build/$HOST_TRIPLE/kasld"
33+
if [ ! -x "$BIN" ]; then
34+
printf '%ssysroot-containment guard: SKIP%s (no host binary at %s; run make first)\n' \
35+
"$YELLOW" "$RESET" "$BIN"
36+
exit 0
37+
fi
38+
39+
# Neither root exists, so neither can supply a fact. The long one is sized past
40+
# KASLD_PATH_MAX (4096) once any fact path is appended; awk builds it because a
41+
# 4091-character literal is not something to keep in a source file.
42+
SHORT=/nonexistent-sysroot
43+
LONG=$(awk 'BEGIN { s = "/"; while (length(s) < 4091) s = s "aaaaaaaaaa";
44+
print substr(s, 1, 4091) }')
45+
46+
# Facts a run actually read: --verbose echoes each component's wire lines, and
47+
# every fact a component emits is one of them.
48+
facts() { # $1 = KASLD_SYSROOT value, empty for a live run
49+
if [ -z "$1" ]; then
50+
"$BIN" -q -v 2>&1 | grep -c '^[VPSR] '
51+
else
52+
KASLD_SYSROOT="$1" "$BIN" -q -v 2>&1 | grep -c '^[VPSR] '
53+
fi
54+
}
55+
56+
live=$(facts '')
57+
short=$(facts "$SHORT")
58+
long=$(facts "$LONG")
59+
60+
if [ "$((live - short))" -lt 10 ]; then
61+
printf '%ssysroot-containment guard: SKIP%s (host exposes %d facts, empty sysroot %d; too few to detect a fallback)\n' \
62+
"$YELLOW" "$RESET" "$live" "$short"
63+
exit 0
64+
fi
65+
66+
if [ "$long" -ne "$short" ]; then
67+
printf '%ssysroot-containment guard: FAIL%s\n' "$RED" "$RESET"
68+
printf ' an over-long KASLD_SYSROOT read %d facts; a short one reads %d\n' \
69+
"$long" "$short"
70+
printf ' (the live host reads %d) — a read escaped the sysroot\n' "$live"
71+
exit 1
72+
fi
73+
74+
printf '%ssysroot-containment guard: OK%s (over-long and short sysroots both read %d facts; live host %d)\n' \
75+
"$GREEN" "$RESET" "$short" "$live"

0 commit comments

Comments
 (0)