# DF-0677 — sl_compress_init heap overflow via unvalidated max_state

## Verdict: REPRODUCED (heap overflow primitive confirmed + fix validated)

`sl_compress_init(comp, max_state)` (sys/net/ppp_layer/slcompress.c:64-87) does not
clamp `max_state` against `MAX_STATES` (16). The loop

```c
for (i = max_state; i > 0; --i) {
    tstate[i].cs_id = i;            /* controlled byte */
    tstate[i].cs_next = &tstate[i-1]; /* kernel pointer */
}
```

writes `tstate[16..max_state]` **out of bounds** past the 16-entry `tstate[]` array
embedded in `struct slcompress`. The `sppp` caller passes `p[4]` — a raw byte
(0..255) taken verbatim from a remote PPP peer's IPCP Configure-Request compression
option (`sys/net/sppp/if_spppsubr.c:2976`, and the NAK path at `:3167`). `sp->pp_comp`
itself is a `kmalloc(sizeof(struct slcompress), M_TEMP)` (`if_spppsubr.c:964`), so the
overflow corrupts the adjacent M_TEMP kernel heap.

## Primitive characterization (measured on this guest)

| property | value |
|---|---|
| allocation overflowed | `kmalloc(sizeof(struct slcompress)=4656, M_TEMP)` → kmalloc-8192 bucket |
| worst-case overflow | `max_state=255` → writes `tstate[32..255]` past the struct = **~32 KB** (32123-byte span) |
| bytes actually written | 2015 bytes, in scattered 9-byte chunks (8-byte pointer + 1-byte id) at a **144-byte stride** |
| content control | per chunk: a **kernel pointer** (`cs_next=&tstate[i-1]`, heap-relative, predictable w/ KASLR off) and a **controlled byte** (`cs_id = i & 0xff`, attacker-chosen via max_state) |
| immediate effect | **no panic** — overflow lands in mapped slab/kmem pages, corruption planted silently |

Decisive harness evidence (`dmesg` after `kldload slc_oob.ko`, baseline/unfixed):
```
DF0677: RESULT=OVERFLOW_DETECTED  2015 guard bytes clobbered, farthest write at +32123 bytes past struct
DF0677: guard[0..15]: a0 d1 44 18 01 f8 ff ff aa aa 20 aa aa aa aa aa
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^         ^^
                       kernel pointer 0xfffff8011844d1a0   cs_id=0x20 (=32, attacker-controlled)
```

## Reachability & threat model

- **Trigger path (as filed): remote, unauthenticated PPP peer.** `sppp` reaches
  `PHASE_NETWORK` (and thus IPCP) without authentication when no LCP auth option is
  negotiated (`if_spppsubr.c:2588-2592`); VJ compression is enabled by default
  (`CONF_ENABLE_VJ`, `if_spppsubr.c:959`). A single crafted IPCP Configure-Request
  (option type=2 IPCP compression, proto=0x002d VJ, max_state=255) reaches the sink.
  Common on serial PPP, PPPoE, leased lines.
- **Local reachability:** the same `sl_compress_init` is also called from the
  **netgraph `ng_vjc`** node (`sys/netgraph7/vjc/ng_vjc.c:316/322`) with
  `c->maxChannel` from a `NGM_VJC_SET_CONFIG` control message — reachable from
  userspace via `ng_socket` if that module is loadable by the caller (root-only
  `kldload`, but ng_socket control itself is not privilege-gated). This is a
  potential local-unprivileged path; not exercised in this run.

## Escalation ceiling / why no `uid=0` line

This is a **remote**-peer-driven memory-corruption bug, not a local-unprivileged
syscall bug. The "local unprivileged user → uid=0" primary question does not map
cleanly: locally instantiating a `sppp` interface (the filed trigger) requires root
(`ifconfig create`), and the attacker is the *remote* PPP peer. Given the guest has
**no SMAP / no SMEP / no KASLR**, the realistic ceiling is **remote kernel code
execution**: the 32 KB attacker-influenced heap write (controlled byte + predictable
heap pointers, fixed 144-byte stride) is a strong grooming primitive — a remote
attacker who can hold the PPP link open can shape adjacent M_TEMP slabs, corrupt a
victim object holding a function pointer / `ucred *`, and redirect it to
userspace-resident shellcode (no SMEP) that escalates. Full remote heap-grooming →
code-exec was not developed in this session (it requires a live PPP-link harness);
the primitive is, however, fully demonstrated and characterized above. A local
escalation via the ng_vjc path is the most promising follow-up.

## PoC changes

- Wrote `slc_oob.c` + `Makefile`: a loadable harness that replicates sppp's exact
  allocation (`kmalloc(sizeof(struct slcompress), M_TEMP)`) and calls
  `sl_compress_init(comp, 255)`, using a 64 KB guard region + 0xAA sentinel to
  detect the OOB deterministically and hexdump the first written chunk. It compiles
  the real `slcompress.c` from the source tree (no dependency on the unloaded
  sppp/sl module), so it exercises the actual vulnerable code.
- `build.sh` / `run.sh`: `make` then `kldload ./slc_oob.ko`.

## Fix (fix.diff)

Clamp `max_state` to `[0, MAX_STATES-1]` after the `max_state == -1` block, before
the loop, covering all callers (sppp `p[4]`, ng_vjc `c->maxChannel`, and any future
caller) and the negative-`int` case (which would otherwise wrap a `u_int` loop index):

```c
if (max_state < 0 || max_state > MAX_STATES - 1)
    max_state = MAX_STATES - 1;
```

This **matches the finding proposal's intent** (clamp before the loop) but is placed
as a single clamp point rather than inside the `else` branch, so it also protects the
`max_state == -1` re-set path and negatives. Validated: baseline `OVERFLOW_DETECTED`
(2015 bytes, pointer+0x20) → fixed `NO_OVERFLOW` (guard untouched).

## Kernel references (verified)
- `sys/net/ppp_layer/slcompress.c:64` — `sl_compress_init(comp, max_state)`
- `sys/net/ppp_layer/slcompress.c:69` — only `== -1` is special-cased, no clamp
- `sys/net/ppp_layer/slcompress.c:77-79` — the unbounded `tstate[i]` loop
- `sys/net/slcompress.h:47` — `#define MAX_STATES 16`
- `sys/net/slcompress.h:153` — `struct cstate tstate[MAX_STATES]`
- `sys/net/sppp/if_spppsubr.c:964` — `sp->pp_comp = kmalloc(sizeof(struct slcompress), M_TEMP, ...)`
- `sys/net/sppp/if_spppsubr.c:2976,3167` — `sl_compress_init(sp->pp_comp, p[4])` (unvalidated byte)