# DF-2227 — unbounded `while (*cp != '"')` scan in XML attribute-value parser

## Verdict
**REPRODUCED** — the code claim is **CONFIRMED**: the attribute-value scan loop at
`sys/libprop/prop_object.c:484-485` (`while (*cp != '"') cp++;`) has **no NUL
check in its body**, and the kernel copyin path NUL-terminates attacker XML at
exactly `buf[pref_len]` (`prop_kern.c:404`). The pre-loop `_PROP_EOF` check at
line 480 only verifies the **first** byte after the opening quote — it does not
bound the subsequent scan. A plist whose attribute value opens a `"` but never
closes it (e.g. `<plist version="X` with `pref_len == strlen`) drives the scan
**past the NUL sentinel** into adjacent kernel memory: `'\0'` (0x00) ≠ `'"'`
(0x22), so the loop advances and reads uninitialised/unmapped kernel heap.
**Confirmed as a kernel panic** (page fault in the buggy instruction itself).
The proposed **fix was VALIDATED** on a single-fix kernel (panic → clean EIO,
guest stays up; legitimate-XML parse path unchanged).

## Impact (honest, on the default GENERIC guest)
**Privileged kernel DoS (panic via page fault) + latent OOB read.** NOT an
unprivileged escalation — two independent **valid hard blockers** apply (see
Phase 6):
1. **Root-only reachability.** All four kernel callers of
   `prop_dictionary_copyin_ioctl()` are privileged on the default guest
   (re-verified here, consistent with DF-2231):
   - `sys/kern/kern_udev.c:892` `UDEVPROP` on `/dev/udev` → node is
     `root:wheel 0600` (`maxx` gets `EPERM`). **This is the only caller compiled
     into the base kernel** and is what the PoC exercises (as root).
   - `sys/kern/vfs_quota.c:346` `vquotactl(530)` → gated by `vfs_quota_enabled`
     (default 0, `CTLFLAG_RD`); `sysctl vfs.quota.enabled` is unset on this guest.
   - `sys/dev/disk/dm/device-mapper.c:267` `NETBSD_DM_IOCTL` → dm module not
     loaded (would be `0640 root:operator`).
   - `sys/dev/misc/tbridge/tbridge.c:258` `TBRIDGE_LOADTEST` → module not loaded.
   => No **unprivileged** trigger exists. root→kernel is game-over by definition;
      this is a privileged DoS / hardening gap, not unpriv→root.
2. **Read-only primitive.** This is an OOB *read* (`cmpb $0x22,(%rax)`), not a
   write — there is no corruption primitive to convert into control flow, so no
   escalation chain is derivable regardless of reachability.

The finding's secondary "kernel heap leak via copyout" claim is **not realised on
this path**: after the runaway scan (if it stops on a `0x22` byte in adjacent
heap rather than faulting), the very next checks at `prop_object.c:486-492`
almost always return `false` (the byte after the stray `0x22` is not `>`), so
`_prop_object_internalize_find_tag` returns false →
`prop_dictionary_internalize` returns `NULL` → `EIO`. No prop objects are
constructed from the OOB bytes, so nothing is copied back to userspace. The
realistic ceiling is **privileged kernel DoS**.

## Mechanism (trigger → primitive → effect)
1. **Trigger.** A privileged caller (`/dev/udev` `UDEVPROP`, root) issues an
   ioctl whose `ap->a_data` is a user `struct plistref`
   `{ void *pref_plist; size_t pref_len; }` (`sys/libprop/plistref.h:43-45`).
2. **Sink.** `udev_dev_ioctl()` (`kern_udev.c:892`) →
   `prop_dictionary_copyin_ioctl()` → `_prop_object_copyin()` (`prop_kern.c:398`):
   ```c
   buf = kmalloc(pref->pref_len + 1, M_TEMP, M_WAITOK);   /* 398 */
   error = copyin(pref->pref_plist, buf, pref->pref_len);  /* 399 */
   ...
   buf[pref->pref_len] = '\0';                             /* 404 — sole terminator */
   ```
   The NUL sentinel lands at exactly `buf[pref_len]` — the **last** allocated byte.
3. **Buggy scan** (`prop_object.c` `_prop_object_internalize_find_tag`,
   parsing the `<plist version="...">` tag):
   ```c
   cp++;                          /* 479: cp -> byte right after opening '"' */
   if (_PROP_EOF(*cp))            /* 480: ONLY checks the FIRST post-quote byte */
       return (false);
   ctx->poic_tagattrval = cp;     /* 483 */
   while (*cp != '\"')            /* 484: *** NO NUL CHECK IN BODY *** */
       cp++;                      /* 485: 'X'->cp++, '\0'(0x00!=0x22)->cp++ = OOB! */
   ```
   Every *other* scanner in this function bounds itself with `_PROP_ISSPACE`
   (which includes `_PROP_EOF`, i.e. stops at NUL): lines 379, 423, 462, 469.
   Only the attribute-value loop at 484 uses the bare `*cp != '"'` test, so it
   sails past the NUL sentinel.
4. **Effect.** `cp` now points past the allocation. The loop keeps reading
   adjacent kernel VA one byte at a time until it either finds a `0x22` byte
   (parse then fails cleanly → EIO, silent OOB read) or walks into an unmapped
   page → **page fault → kernel panic**.
   - Reproduced reliably as a **panic** with a large `pref_len` (60000): the
     `kmalloc(60001)` allocation goes through the kmem/page allocator, so the
     byte immediately after `buf[pref_len]` sits at a page boundary that is
     typically unmapped → fault on the first OOB read. Panic signature (2/2 runs):
     ```
     Fatal trap 12: page fault while in kernel mode
     fault virtual address = 0xfffff8011864f000   (page-aligned, 1st byte past buffer)
     fault code = supervisor read data, page not present
     Stopped at _prop_object_internalize_find_tag+0x32f: cmpb $0x22,(%rax)
     ```
     The faulting instruction `cmpb $0x22,(%rax)` **is** the body of the loop
     at line 484 — definitive attribution.
   - With a small `pref_len` (17 → `kmalloc-32` slab bucket) the scan usually
     finds a `0x22` byte in adjacent slab memory first → silent OOB read → EIO.

## Exploit chain
Not applicable (read-only primitive + root-only reachability = two valid hard
blockers per Phase 6; no escalation chain is derivable). Impact ceiling is
privileged kernel DoS (panic). No `exploit.c`/`chain.c` authored — the trigger
PoC (`oob_read_poc.c`) is the deliverable and demonstrates the panic.

## PoC changes
Rewrote the seeded `oob_read_poc.c` (which targeted the not-loaded dm
`/dev/mapper/control`) into a parameterised privileged demonstrator via the
**base-kernel** `/dev/udev` `UDEVPROP` ioctl:
- Uses the real `<sys/udev.h>` `UDEVPROP` macro + `<libprop/plistref.h>`.
- `mmap`s a user buffer of `pref_len` bytes: `<plist version="` + `'A'` padding,
  **no closing quote, no `>`** — so the `prop_kern.c:404` NUL is the sole
  terminator and lands right after the unclosed value.
- `argv[1]` selects `pref_len`: `60000` (default → `kmalloc` page-zone → reliable
  panic) or `17` (slab bucket → silent OOB read → EIO).
- Added `build.sh`/`run.sh`. Original README claim ("kernel panic") is **correct**
  for the large-buffer mode; added the honest silent-OOB variant for the slab mode.

## Fix (validated)
Add the per-iteration `_PROP_EOF(*cp)` check to the loop body — the same idiom
every other scanner in this function already uses — and treat the unterminated
case as a parse error. Full diff in `fix.diff`:
```c
-	while (*cp != '\"')
+	while (!_PROP_EOF(*cp) && *cp != '\"')
 		cp++;
-	if (_PROP_EOF(*cp))
+	if (_PROP_EOF(*cp) || *cp != '\"')
 		return (false);
```
`git apply --check` against `sys/`: **CLEAN** (1 file, 2+2 lines).

### Fix validation (single-fix kernel, `make -j6 nativekernel`)
- **Baseline `#0`** (unpatched, Jul 2 2026): `/tmp/oob2227 60000` → **kernel
  PANIC** (`_prop_object_internalize_find_tag+0x32f: cmpb $0x22,(%rax)`, page
  fault @ `0xfffff8011864f000`, guest DOWN). Reproduced 2/2.
- **Single-fix `#1`** (Aug 8 16:51:23 2026, sha256
  `350f0274…`): `/tmp/oob2227 60000` → **clean `errno=5 EIO`**, NO panic, guest
  UP, 0 new `Fatal trap` in boot.log. 3/3 large + 1/1 small variant all clean.
  The fix's `_PROP_EOF(*cp)` check stops the scan at the `buf[pref_len]` NUL
  sentinel; the unterminated quote is then detected as a parse error.
- **Legit-XML path intact** on `#1`: a well-formed `<plist version="1.0">
  <dict></dict></plist>` still parses (returns `EINVAL` from the udev handler —
  parse succeeded, handler ran, no `"command"` key — i.e. the parser is not
  broken by the fix; only the malformed unclosed-quote input now fails cleanly).

## Files
- `oob_read_poc.c` — parameterised privileged demonstrator via `/dev/udev UDEVPROP`.
- `build.sh` / `run.sh` — exact build/run (`run.sh [pref_len]`, default 60000).
- `build.log` — final clean userspace build.
- `run.log` — baseline (#0) panic run summary.
- `panic.txt` — full panic signature from `dfbsd-qemu/boot.log` (the crash proof).
- `fix_build.log` — single-fix `nativekernel` build output (`NK_DONE rc=0`).
- `fix_run.log` — patched (#1) clean-run summary (3× large + small, all EIO, guest up).
- `env.txt` — guest `uname`, `cc`, `/dev/udev` perms, `vfs.quota.enabled`.
- `fix.diff` — `git apply`-able fix (per-iteration NUL check).
- `manifest.json` — machine-readable catalog.