DF-0596 — PoC: SMP race on ng_pptpgre xmitWin -> timeSent[] OOB write

Remote (PPTP-WAN-reachable) race PoC. The legacy ng_pptpgre node has no per-node serialization; ng_send_data dispatches rcvdata inline on the caller's CPU. Two GRE ack packets arriving on different CPUs execute ng_pptpgre_recv concurrently. The xmitWin read-check-increment at ng_pptpgre.c:672-676 is a TOCTOU; two concurrent ack handlers can both see xmitWin == 15, both pass the < 16 check, both increment → xmitWin == 17. Subsequent xmit then permits timeSent index 16 (one past the end of pptptime_t timeSent[16]) → 8-byte heap overflow into recvSeq/xmitSeq.

Files

• race.c — sketch driver (4 threads hammering the same GRE ack to maximize TOCTOU overlap on the xmitWin growth window).
• (added by per-PoC verifier) full race.c rewrite with proper PPTP session bring-up (TCP 1723 control channel for CID discovery, then the burst), build.sh, run.sh, build.log, run.log, VERDICT.md, manifest.json, fix.diff.

Target setup (DragonFlyBSD)

# load netgraph modules
kldload ng_socket ng_ksocket ng_pptpgre ng_iface ng_eiface

# configure a PPTP concentrator (mpd5 or a custom ngctl script). Ensure:
#   - the guest has >= 2 vCPUs
#   - ng_pptpgre node is reachable on the WAN
#   - xmitWin has grown to PPTP_XMIT_WIN-1 = 15 (~15 successful ack
#     round-trips during PPP negotiation; happens naturally)

Build & run (Linux attacker)

cc -O2 -lpthread -o race race.c
./race <server_wan_ip> <cid> <ack_value>

ack_value must be at or beyond the current winAck threshold for the server to accept the ack for window growth (i.e. reflect the next window- grow point). On a fresh session this advances naturally; for a targeted trigger, the verifier should sniff the live session's last-seen ack sequence and use ack = winAck + delta values that pass the PPTP_SEQ_DIFF(ack, a->winAck) >= 0 check at :672.

Expected first outcome

After the race succeeds (xmitWin == 17), subsequent server xmit trips the OOB write at ng_pptpgre.c:514. The server's recvSeq is overwritten with a large timestamp value. All subsequent received GRE data packets fail the sequence check at :691 and are silently dropped (recvOutOfOrder++). The PPTP session is dead.

On DEBUG kernels, the corrupted sequence numbers may trigger KASSERT failures:

panic: ... KASSERT in ng_pptpgre_xmit / ng_pptpgre_recv
backtrace: ng_pptpgre_xmit+0x... ng_pptpgre_recv+0x... ng_send_data+0x...

Notes for the per-PoC verifier

• The race window is narrow (~10 instructions, lines :672-676); expect O(1000-10000) attempts on a 2-vCPU guest under moderate load. The 4-thread burst maximizes overlap probability.
• The CID can be obtained by establishing a legitimate PPTP session (TCP 1723 control channel) or brute-forced (only 65536 possibilities, but each wrong guess wastes server resources and may reveal the wrong-CID error).
• The overflow stays within the single priv kmalloc allocation (~200 bytes remaining after the write point), so it does not directly corrupt adjacent slab objects — the demonstrated impact is PPTP session DoS, not code execution. If heap grooming of a same-sized slab yields a controlled victim object, document it in VERDICT.md; otherwise the verdict should reflect session_DoS_confirmed / escalation_unverified and the finding stays Medium.
• Verify the fix with git apply findings/poc/DF-0596/fix.diff (the per-node spinlock patch in the finding markdown); after the fix the race should no longer fire.
• Same class as DF-0590 (legacy ng_bridge no-serialization races): the entire legacy netgraph tree lacks per-node SMP locking. Flagged for the maintainer as a systemic issue.

DF-0596 — VERDICT: Unsynchronized SMP race on xmitWin in ng_pptpgre

Verdict: REPRODUCED (code-level) — TOCTOU race confirmed; OOB write contingent on codegen

Finding: The legacy ng_pptpgre netgraph node has no per-node serialization (no spinlock, token, or mutex) protecting its mutable state (xmitWin, timeSent[], winAck, recvAck, rtt, dev, ato, recvSeq, xmitSeq). On an SMP system, two GRE packets can be processed concurrently on different CPUs, each invoking ng_pptpgre_recv on the same node without synchronization.

Root Cause Confirmation (source trace)

1. No serialization — ng_pptpgre_recv (sys/netgraph/pptpgre/ng_pptpgre.c:566) accesses all shared state without any lock. ng_send_data (sys/netgraph/netgraph/ng_base.c:1678) dispatches rcvdata inline on the caller's CPU. The ksocket upcall uses crit_enter() which is per-CPU only. Confirmed: no serialization exists.

2. TOCTOU on xmitWin growth (ng_pptpgre.c:672-676): c if (PPTP_SEQ_DIFF(ack, a->winAck) >= 0 && a->xmitWin < PPTP_XMIT_WIN) { // CHECK (read) a->xmitWin++; // INCREMENT (non-atomic RMW) a->winAck = ack + a->xmitWin; } xmitWin is u_int16_t at struct offset 0x2c in ng_pptpgre_ackp (ng_pptpgre.c:146). The read-check-increment is non-atomic. Two concurrent ack handlers can both see xmitWin == 15, both pass < 16, and both increment.

3. OOB write target (ng_pptpgre.c:514): c a->timeSent[priv->xmitSeq - priv->recvAck] = ng_pptpgre_time(node); The index is xmitSeq - recvAck, bounded by xmitWin. With xmitWin > 16, the index can reach 16, one past the end of pptptime_t timeSent[PPTP_XMIT_WIN] (16 elements, valid indices 0..15). timeSent is the last field of struct ng_pptpgre_ackp (ng_pptpgre.c:152), followed immediately by recvSeq in struct ng_pptpgre_private (ng_pptpgre.c:165).

4. Struct layout verified — harness confirms &timeSent[16] and &recvSeq are both at offset 200 in struct ng_pptpgre_private. An 8-byte write at timeSent[16] would overwrite recvSeq (4 bytes) and xmitSeq (4 bytes).

Harness Results

Baseline — TOCTOU is winnable

=== -O0 (separate load/add/store codegen — maximizes race window) ===
[threads=2] xmitWin>16 in 31 rounds, max_xw=17
[threads=4] xmitWin>16 in 70 rounds, max_xw=17
Total over-16 rounds: 104/30000, max_xw=17
*** CONFIRMED: xmitWin can exceed PPTP_XMIT_WIN -> timeSent[] OOB ***

Baseline — default kernel codegen (-O2)

=== -O2 (single load reused for check+increment — compiler prevents exceeding 16) ===
[threads=2/4/8] xmitWin>16 in 0 rounds, max_xw=16
Race not won (xmitWin stayed <= 16). Codegen prevents exceeding the bound.

Kernel disassembly (unpatched module, -O2)

The compiler generates a single load for both check and increment:

1179: movzwl 0x2c(%rbx),%eax   ; LOAD xmitWin once
117d: cmp    $0xf,%ax           ; CHECK (reuses register)
1183: add    $0x1,%eax          ; INCREMENT (reuses register, NOT reload)
1186: mov    %ax,0x2c(%rbx)     ; STORE

With this codegen, two concurrent threads both compute 15+1=16 from their stale register copy. xmitWin cannot reach 17 with -O2.

Impact Assessment

What IS confirmed:

• Missing serialization is a genuine bug — all shared state in ng_pptpgre_ackp and ng_pptpgre_private is accessed without any lock from concurrent CPU contexts.
• TOCTOU on xmitWin is real — the read-check-increment race is winnable (proven with -O0 harness: 104/30000 rounds reaching xmitWin=17).
• Struct layout confirms OOB target — timeSent[16] overlaps recvSeq/xmitSeq at offset 200 in the priv struct.
• Other unsynchronized data races are real and dangerous:
• bcopy(a->timeSent + index + 1, a->timeSent, ...) at line 668 — two concurrent overlapping bcopy operations on the same buffer is undefined behavior.
• priv->recvAck = ack at line 652 — last writer wins.
• a->rtt += ... at line 657, a->dev += ... at line 660 — lost updates.
• priv->recvSeq = seq at line 698 — last writer wins.

What is NOT confirmed on the default kernel:

• The specific heap OOB write at timeSent[16] is not achievable with the default -O2 kernel build. The compiler reuses the register value from the check, so xmitWin stays at 16 (valid maximum). The OOB IS achievable with -O0 codegen (harness proof), demonstrating the code is inherently unsafe.

Realistic impact:

• PPTP session DoS from corrupted state (recvSeq, rtt, ato, timeSent bcopy races) — severity Medium (deprecated protocol, legacy systems only).
• Heap OOB write is latent — not reachable with current -O2 codegen, but any compiler change, LTO configuration, or future code modification could expose it. The bounds check fix is appropriate defense-in-depth.

PoC Changes

• Rewrote race.c → race_harness.c: The original PoC was a Linux raw-socket sketch that couldn't run on the guest (no PPTP concentrator, no external GRE path). Replaced with a deterministic pthread-based harness that replicates the exact kernel growth logic, struct layout, and race conditions. The harness proves: 1. Struct layout: timeSent[16] overlaps recvSeq (OOB target). 2. TOCTOU is winnable with -O0 (xmitWin reaches 17 in ~0.3% of rounds). 3. TOCTOU is NOT winnable with -O2 (compiler prevents exceeding 16).
• Added race_harness_fixed.c: Same harness with the fix applied (clamp + bounds check), showing zero OOB even with -O0 codegen.

Fix (fix.diff)

Two-part defense-in-depth fix:

1. Bounds check on timeSent index (ng_pptpgre.c:514): Wrap the array access in an explicit bounds check: c u_int32_t _ts_idx = priv->xmitSeq - priv->recvAck; if (_ts_idx < PPTP_XMIT_WIN) a->timeSent[_ts_idx] = ng_pptpgre_time(node); This prevents the OOB write regardless of what xmitWin does.

2. Defensive clamp on xmitWin (ng_pptpgre.c:674): After the increment, clamp to PPTP_XMIT_WIN: c if (a->xmitWin > PPTP_XMIT_WIN) a->xmitWin = PPTP_XMIT_WIN; (Note: gcc -O2 optimizes this away as dead code since the preceding check already prevents exceeding 16; it's included for defense-in-depth and future-proofing.)

A more complete fix would add a per-node spinlock to serialize all ack-processing state modifications (the finding markdown's recommendation), but this is complex due to reentrancy (ng_pptpgre_xmit is called from ng_pptpgre_recv). The bounds check + clamp is the minimal targeted fix that eliminates the specific OOB write vulnerability.

Fix Validation

Fix status: FIXED — the bounds check prevents the OOB write even under adversarial codegen conditions (-O0). The defense-in-depth clamp prevents xmitWin from exceeding PPTP_XMIT_WIN.