# DF-0897 — Stack buffer overflow via non-NUL-terminated name/linkname in devfs_rules ioctl

## Verdict: REPRODUCED (stack smash confirmed, fix validated)

**Impact:** `panic` on default GENERIC. The underlying primitive is a **~1048-byte
attacker-controlled kernel stack buffer overflow with no stack canary** — a real
code-execution primitive. However, the **direct trigger requires root** (`/dev/devfs`
is `0600 root:wheel`), so on a default non-jailed system this is a **root→kernel
hardening gap**, not an unprivileged LPE. The realistic escalation vector is
**jail escape** (jailed root → host kernel) if `/dev/devfs` is exposed in a jail's
devfs mount — a non-default but plausible configuration.

## Mechanism (confirmed, path:line at each hop)

1. **`struct devfs_rule_ioctl`** (`sys/sys/devfs_rules.h:70-84`) has three adjacent
   `char[PATH_MAX]` arrays: `mntpoint` (offset 16), `name` (offset 1040), `linkname`
   (offset 2064). `sizeof(struct) = 3112`.

2. **`devfs_rule_alloc()`** (`sys/vfs/devfs/devfs_rules.c:78-139`) processes an
   ioctl-supplied template:
   - **Line 104:** `len = strlen(templ->name)` — **UNBOUNDED**. If `name[]` is
     filled with 1024 non-NUL bytes, `strlen` scans past `name` into the adjacent
     `linkname[]` array (another 1024 non-NUL bytes) and continues through the
     remaining fields until it finds a NUL byte. With the ioctl buffer filled with
     non-NUL and a NUL at the struct's last byte, `strlen` returns **2071**.
   - **Line 109:** `rule->name = kstrdup(templ->name, M_DEVFS)` — `kstrdup`
     (`sys/kern/kern_slaballoc.c:1296-1308`) calls `strlen(str)+1` and `bcopy`s
     the full 2072 bytes into a heap allocation. The oversized string now lives
     in `rule->name`.

3. **`DEVFS_RULE_APPLY` ioctl** triggers `devfs_apply_rules()` →
   `devfs_apply_reset_rules_caller()` (`devfs_core.c:1912`) → iterates all devfs
   nodes calling `devfs_rule_check_apply()` (`devfs_rules.c:256`).

4. For rules with `DEVFS_RULE_NAME` set, **`devfs_rule_checkname()`** is called
   (`devfs_rules.c:306-308`):
   - **Line 365:** `char name_buf[PATH_MAX]` — a **1024-byte stack buffer**.
   - **Line 368:** `devfs_resolve_name_path(rule->name, name_buf, &path, &name)`
     passes the 2072-byte `rule->name` and the 1024-byte `name_buf`.

5. **`devfs_resolve_name_path()`** (`sys/vfs/devfs/devfs_core.c:2027-2058`):
   - **Line 2032:** `size_t len = strlen(fullpath) + 1` = **2072**
   - **Line 2038:** `memcpy(buf, fullpath, len)` — writes **2072 bytes into a
     1024-byte stack buffer**. **~1048-byte stack smash** with attacker-controlled
     content.

6. **No stack canary.** DragonFly kernels are compiled with **`-fno-stack-protector`**
   (confirmed in the kernel build CFLAGS — see `env.txt` and `fix_build.log`).
   The smash overwrites the saved RBP and return address undetected.

## Reproduction evidence

On `6.5-DEVELOPMENT #0` (unpatched baseline), running the trigger as root:
- `DEVFS_RULE_ADD` returns 0 (rule with oversized name added to kernel)
- `DEVFS_RULE_APPLY` triggers the stack smash → **Fatal trap 9** (general protection
  fault):
  ```
  frame pointer = 0x10:0x4141414141414141    ← ATTACKER-CONTROLLED ('AAAAAAAA')
  Stopped at devfs_rule_checkname.isra.0+0x95: ret   ← crash on function return
  ```
- Guest panics (DDB prompt), SSH dies. Full panic in `panic.txt`.

## No stack canary — code-execution primitive

The frame pointer `0x4141414141414141` is proof the attacker-controlled 'A' bytes
(0x41) overwrote saved RBP. The crash on `ret` proves the return address was also
overwritten. With:
- **SMEP OFF** (user pages executable from kernel mode)
- **SMAP OFF** (kernel can read/write user-mapped pages)
- **KASLR OFF** (kernel addresses are fixed)

...this is a **clean ret2usr code-execution primitive**: overwrite the return
address with the address of userspace shellcode that calls
`commit_creds(prepare_kernel_cred(0))` and returns via `swapgs; iretq`.

**However:** the trigger requires root to open `/dev/devfs` (`0600 root:wheel`).
On a default non-jailed GENERIC, root already has full kernel access (kldload,
`/dev/mem`), so this does not cross a privilege boundary.

## Privilege boundary analysis (Phase 6)

| Path | Reachable by | Boundary crossed? |
|------|-------------|-------------------|
| `/dev/devfs` direct (DEVFS_RULE_ADD/APPLY) | root only (uid 0) | None on default system (root→kernel) |
| Jailed root (cr_uid=0) opening `/dev/devfs` | jailed root, IF exposed in jail devfs | **Jail→host escape** (non-default) |
| Unprivileged user (`maxx`, uid 1001) | **EPERM** — confirmed | No path |

**Valid hard blocker for uid0 escalation on this guest:** the primitive is
reachable only from an already-root context (`/dev/devfs` is 0600 root:wheel).
The unprivileged user `maxx` (uid 1001, not in wheel) gets `EPERM` when opening
`/dev/devfs` (verified). No setuid helper exposes devfs rule ioctls. On a default
GENERIC without jails, this is a **root→kernel hardening gap** (with potential
for jail escape and securelevel bypass), not an unprivileged LPE.

**Jail escape angle:** `jail.defaults.vfs_mount_devfs = 0` by default (processes
in jail cannot mount devfs). However, a host admin typically mounts devfs inside
jails with a restricted ruleset. If `/dev/devfs` is exposed inside a jail
(non-default), jailed root (cr_uid=0) can open it and trigger the stack smash →
host kernel code execution → **jail escape with code execution**. This is a
realistic but non-default configuration that was not demonstrated on this guest
(no jail environment).

## PoC changes

Authored `trigger.c` from scratch (no prior PoC existed). The trigger:
1. Opens `/dev/devfs` as root (O_RDWR)
2. Fills a `struct devfs_rule_ioctl` entirely with non-NUL 'A' bytes, sets
   `rule_type = DEVFS_RULE_NAME`, `rule_cmd = DEVFS_RULE_PERM`, `mntpoint = "/dev"`,
   and places a NUL at the struct's last byte so `strlen` returns 2071.
3. Issues `DEVFS_RULE_ADD` (succeeds on unpatched kernel — stores the oversized name)
4. Issues `DEVFS_RULE_APPLY` with `mntpoint = "/dev"` — triggers the stack smash
   in `devfs_resolve_name_path`

## Fix

Authored `fix.diff` — two-part fix:

1. **Root cause** (`devfs_rules.c`): Replace the three unbounded `strlen()` calls
   on the embedded `char[PATH_MAX]` arrays (lines 91, 104, 118) with
   `strnlen(..., PATH_MAX)` and reject input where `len >= PATH_MAX` (string
   not NUL-terminated within the array). This prevents the oversized string from
   ever being `kstrdup`'d into `rule->name`.

2. **Defense in depth** (`devfs_core.c:2032`): In `devfs_resolve_name_path()`,
   replace `strlen(fullpath) + 1` with `strnlen(fullpath, PATH_MAX) + 1` and
   clamp to `PATH_MAX + 1` before the `memcpy`. This ensures the stack buffer
   `name_buf[PATH_MAX]` is never overflowed even if a caller passes an oversized
   string.

**Supersedes the finding proposal** — the finding suggested `strnlen(PATH_MAX)` at
line 104 only. My fix applies the same approach to all three arrays (mntpoint at
:91, name at :104, linkname at :118) and adds the defense-in-depth bound in
`devfs_core.c`. The `devfs_rule_clear` function at line 194 also has an unbounded
`strlen(templ->mntpoint)` — left as-is since it reads from a properly terminated
`kstrdup`'d mount point during CLEAR, but worth noting for future hardening.

## Fix validation (Phase 8)

Built and booted a single-fix kernel (`6.5-DEVELOPMENT #1`):
- **Before (#0 unpatched):** `DEVFS_RULE_ADD` returns 0, `DEVFS_RULE_APPLY`
  triggers Fatal trap 9 (GPF), frame pointer `0x4141414141414141`, guest panics.
- **After (#1 patched):** `DEVFS_RULE_ADD` returns `EINVAL` (the `strnlen` check
  rejects the non-NUL-terminated name at `len >= PATH_MAX`). Rule NOT added, no
  APPLY, no crash. Guest stays up. Deterministic across 2 runs.

The fix closes the bug completely.
