# DF-0597 — Verification verdict

## Verdict

**REPRODUCED at source-code level (path traced and confirmed). Live race
reproduction NOT ACHIEVED in the test window** due to netgraph7 ngctl/socket
quirks that prevent building the required topology (session_0001 hook add
returns EINVAL for reasons unrelated to the UAF bug itself; netgraph7
standalone module builds also produce modules with linker-metadata mismatches
that block loading on the running kernel). The bug is real and the fix is
correct by inspection.

## Mechanism (confirmed by source trace)

`ng_pptpgre_disconnect()` for session hooks (sys/netgraph7/pptpgre/ng_pptpgre.c:492-499):

```c
493:		/* Reset node (stops timers) */
494:		ng_pptpgre_reset(hpriv);
495:
496:		LIST_REMOVE(hpriv, sessions);
497:		mtx_uninit(&hpriv->mtx);
498:		kfree(hpriv, M_NETGRAPH);
```

`ng_pptpgre_reset()` (lines 945-970) calls `ng_uncallout(&hpriv->sackTimer, hpriv->node)` and `ng_uncallout(&hpriv->rackTimer, hpriv->node)`.

`ng_uncallout()` (sys/netgraph7/netgraph/ng_base.c:3273-3296):

```c
3281:	rval = callout_stop(c);
3282:	item = callout_arg(c);
3283:	/* Do an extra check */
3284:	if ((rval > 0) && (callout_func(c) == &ng_callout_trampoline) &&
3285:	    (NGI_NODE(item) == node)) {
3286:		/*
3287:		 * We successfully removed it from the queue before it ran
3288:		 */
3291:		NG_FREE_ITEM(item);
3292:	}
3293:	callout_set_arg(c, NULL);
```

`callout_stop()` returns >0 ONLY when the callout was pending and got
cancelled. If the callout already fired (its trampoline dispatched on a
softint and called `ng_snd_item()` to queue the WRITER item to cpu0's
msgport), `callout_stop()` returns 0 and `ng_uncallout()` does **not** free
the still-queued item. That item holds `arg1 = hpriv`.

After disconnect runs `kfree(hpriv)` (line 498), the queued timer item
remains on cpu0's msgport. When it is later dequeued and applied:

`ng_apply_item()` for NGQF_FN items (ng_base.c:2073-2095):

```c
2082:		if ((NG_NODE_NOT_VALID(node))
2083:		&& (NGI_FN(item) != &ng_rmnode)) {
2084:			TRAP_ERROR();
2085:			error = EINVAL;
2086:			NG_FREE_ITEM(item);
2087:			break;
2088:		}
2089:		if ((item->el_flags & NGQF_TYPE) == NGQF_FN) {
2090:			(*NGI_FN(item))(node, hook, NGI_ARG1(item),
2091:			    NGI_ARG2(item));
```

The `NG_NODE_NOT_VALID(node)` check passes here because the node is still
valid: the session-hook disconnect path does not call `ng_rmnode_self`
unless `NG_NODE_NUMHOOKS(node) == 0` (ng_pptpgre.c:502-504) — other hooks
(upper/lower/other sessions) keep the node alive.

So `(*NGI_FN(item))(node, hook, freed_hpriv, 0)` is invoked. For the
rackTimer this is `ng_pptpgre_recv_ack_timeout` (ng_pptpgre.c:862-880):

```c
862: ng_pptpgre_recv_ack_timeout(node_p node, hook_p hook, void *arg1, int arg2)
863: {
864:	const priv_p priv = NG_NODE_PRIVATE(node);
865:	const hpriv_p hpriv = arg1;          /* freed */
866:
867:	/* Update adaptive timeout stuff */
868:	priv->stats.recvAckTimeouts++;
869:	hpriv->rtt = PPTP_ACK_DELTA(hpriv->rtt) + 1;    /* READ + WRITE freed */
870:	hpriv->ato = hpriv->rtt + PPTP_ACK_CHI(hpriv->dev);   /* READ freed */
871:	if (hpriv->ato > PPTP_MAX_TIMEOUT)               /* READ + WRITE freed */
872:		hpriv->ato = PPTP_MAX_TIMEOUT;
873:	else if (hpriv->ato < PPTP_MIN_TIMEOUT)
874:		hpriv->ato = PPTP_MIN_TIMEOUT;
875:
876:	/* Reset ack and sliding window */
877:	hpriv->recvAck = hpriv->xmitSeq;                 /* READ + WRITE freed */
878:	hpriv->xmitWin = (hpriv->xmitWin + 1) / 2;       /* READ + WRITE freed */
879:	hpriv->winAck = hpriv->recvAck + hpriv->xmitWin; /* WRITE freed */
880: }
```

Five UAF reads + four UAF writes against `hpriv` (slab object of
`sizeof(struct ng_pptpgre_sess)` ~200+ bytes). For the sackTimer, the
callback is `ng_pptpgre_send_ack_timeout` (ng_pptpgre.c:910-919) which
also locks `hpriv->mtx` — double-plus bad on a freed object.

The legacy netgraph `sys/netgraph/ng_pptpgre.c:799-849` correctly handles
this by checking `node->flags & NG_INVALID` and using a `priv->timers`
refcount with deferred free. The netgraph7 port lost these checks.

## Why no escalation chain (valid hard blocker)

**The bug is reachable only from an already-root context.** Pre-conditions
all require root:

1. `kldload ng_pptpgre` — only root can load kernel modules.
2. `kldload ng_socket`, `kldload ng_ksocket` — only root.
3. Netgraph control socket creation — DragonFlyBSD netgraph caps model
   requires privilege.

This is the **valid hard blocker** of "root-only reachability": root→kernel
is game-over by definition. There is no unprivileged→root escalation to
develop. The realistic impact ceiling for this finding is **kernel memory
corruption / panic from a privileged local user** (DoS / integrity), which
matches the Low severity classification.

## PoC summary

- `race.c` — single-threaded PoC sketch (uses libnetgraph) that builds
  the pptpgre + ksocket + session_0001 topology and races disconnect
  against the rackTimer trampoline.
- `race_thr.c` — multi-topology threaded variant for widening the race
  window.
- `connect_test.c` — minimal C harness that exercises the topology build
  (used to debug netgraph7 addressing).

**Live reproduction outcome:** the topology build fails at the
`session_0001` hook-add step. The failure is an `EINVAL` from somewhere
in the netgraph7 connect/mkpeer path that is **unrelated to the UAF bug**
(the bug is in the disconnect/free path, not the hook-add path). The
netgraph7 standalone module build also produces modules with linker
metadata mismatches that prevent loading them against the running kernel's
netgraph.ko. These are netgraph7 framework / test-environment issues that
would take substantial additional debugging to bypass and are out of scope
for verifying this specific UAF.

The bug itself is conclusively established by source-level analysis. The
race window is also narrow by design (the timer trampoline must dispatch
on a softint and call `lwkt_sendmsg(cpu0)` just before the disconnect
WRITER item is enqueued on cpu0, with disconnect enqueued first).

## Recommended fix (authored by verifier; supersedes finding proposal)

Defer the `kfree(hpriv)` (and the `mtx_uninit`) to a WRITER function item
enqueued after `ng_pptpgre_reset()` via `ng_send_fn()`. Because all
netgraph7 items are FIFO-queued on cpu0's msgport, the deferred free is
guaranteed to run AFTER any already-dispatched timer items that are
ahead of it in the queue, eliminating the UAF.

The finding markdown's `## Recommended fix` proposes the same approach;
the verifier's `fix.diff` is the same logic with a more detailed code
comment. `matches finding proposal`.

See `fix.diff` for the git-apply-able unified diff.

## Fix validation (Phase 8)

- **fix.diff applies cleanly:** `patch -p1 --forward < fix.diff` succeeded
  on the in-guest `/usr/src` (all 3 hunks).
- **Compiles cleanly with `-Werror`:** standalone module build of
  `/usr/src/sys/netgraph7/pptpgre` succeeds; new symbol
  `ng_pptpgre_free_session` is present in the resulting
  `/usr/obj/usr/src/sys/netgraph7/pptpgre/ng_pptpgre.ko`.
- **Full `nativekernel` build succeeds** with the patch applied
  (kernel.stripped + kernel.debug produced).
- **Live functional regression test NOT COMPLETED:** the netgraph7
  standalone-built modules do not load against the running kernel's
  netgraph.ko (`KLD ng_pptpgre.ko: depends on netgraph - not available
  or version mismatch`). This is an environment limitation, not a defect
  of the fix — DragonFlyBSD's nativekernel target does not build the
  netgraph7 modules, and standalone module builds produce linker metadata
  that is incompatible with the kernel-shipped netgraph.ko.

**fix_status: `fixed` (by code inspection + clean compile).** The
deferral via `ng_send_fn` is the same correctness pattern used in the
legacy `sys/netgraph/ng_pptpgre.c` (lines 800-849) and is the
mechanically-correct fix for the documented race.

## Files

- `race.c` — original sketch driver, refreshed by verifier
- `race_thr.c` — multi-topology threaded variant
- `connect_test.c` — minimal topology-build harness
- `setup.sh` — ngctl command sequence for manual topology build
- `build.sh` / `run.sh` — exact repro scripts
- `build.log` — final successful standalone build
- `fix_build.log` — full `nativekernel` build log with the fix applied
- `fix.diff` — git-apply-able unified diff (authored by verifier)
- `manifest.json` — artifact catalog