# DF-0411 — Infinite loop in ng_parse_skip_value on unclosed quoted string

## Verdict: FALSE POSITIVE (not reproduced — mechanism refuted by code trace + faithful algorithm test)

## Summary
The finding claims that `ng_parse_skip_value()` (sys/netgraph7/netgraph/ng_parse.c:1651-1683)
loops forever when, inside brackets/braces, an unclosed double-quote is
encountered, because `ng_get_string_token()` returns NULL without setting
`*lenp`, leaving `len` stale at 0, so `off += len` never advances and the
same unclosed quote is re-tokenized forever — a permanent kernel-thread hang.

**This mechanism is impossible on the current code.** A line-by-line trace of
`ng_parse_skip_value` + `ng_parse_get_token` + `ng_get_string_token`, confirmed
by a faithful userland copy of all three functions fed the claimed inputs,
shows the loop always terminates.

## Why it does not loop (the proof)

In `ng_parse_skip_value` the loop is:
```c
len = nbracket = nbrace = 0;
do {
    switch (ng_parse_get_token(s, &off, &len)) {   /* sets *lenp on success */
    case T_LBRACKET: nbracket++; break;
    case T_LBRACE:   nbrace++;   break;
    case T_RBRACKET: if (nbracket-- == 0) return EINVAL; break;
    case T_RBRACE:   if (nbrace--   == 0) return EINVAL; break;
    case T_EOF:      return EINVAL;
    default:         break;        /* T_ERROR, T_WORD, T_STRING land here */
    }
    off += len;
} while (nbracket > 0 || nbrace > 0);
```

`ng_parse_get_token` sets `*lenp` for every token type **except** the `T_ERROR`
path (unclosed quote), where `*lenp` is left untouched (ng_parse.c:1718-1719 +
ng_get_string_token:1817 returns NULL without writing `*lenp`). The crucial
fact the finding misses:

| token                          | sets `*lenp` | value |
|--------------------------------|--------------|-------|
| T_LBRACKET / T_LBRACE / `]`,`}`| yes          | 1     |
| T_EQUALS                       | yes          | 1     |
| T_WORD                         | yes          | ≥1    |
| T_STRING                       | yes          | ≥1    |
| T_EOF                          | yes          | 0  ← but this **returns EINVAL immediately** |
| T_ERROR (unclosed quote)       | **no (stale)** | unchanged |

The loop only continues while `nbracket>0 || nbrace>0`. The token that first
makes that true is necessarily `T_LBRACKET` or `T_LBRACE`, which set `len=1`.
From then on, every iteration either (a) calls a successful token that sets
`len≥1`, or (b) hits `T_ERROR` and leaves `len` stale — but the stale value is
**always ≥1** (the most recent successful token set it to ≥1, and only T_EOF
sets it to 0, which returns immediately). Therefore `off += len` always
advances `off` by ≥1; the parser makes forward progress and reaches EOF/`]`/`}`
within a bounded number of iterations.

`len` is 0 in a continuing `off += len` only if a successful token set it to 0
*and* the loop kept going — but the only successful token that sets len=0 is
T_EOF, which returns. So **len is never 0 in a continuing iteration.** QED.

## Empirical confirmation (faithful copy of all 3 kernel functions)

`parsertest.c` is a byte-for-byte copy of the control flow of
`ng_parse_skip_value`, `ng_parse_get_token`, and `ng_get_string_token`
(kmalloc→malloc, kfree→free are the only edits). Run with the finding's exact
trigger and several variants:

```
=== test 0: input="[1 "abc" ===            <- finding's claimed trigger
    iter=0 ... char='[' off_after=1 (delta=1)   len=1  (T_LBRACKET sets nbracket=1)
    iter=1 ... char='1' off_after=2 (delta=1)   len=1  (T_WORD)
    iter=2 ... char='"' off_after=4 (delta=1)   len=1  (T_ERROR; len STALE=1, NOT 0)
    iter=3 ... char='a' off_after=7 (delta=3)   len=3  (T_WORD 'abc')
    iter=4 ... T_EOF -> rc=22 (EINVAL) iters=4   <- TERMINATES
=== test 4: input="[abcdef"abc" ===        <- long word then quote
    iter=2 ... char='"' off_after=13 (delta=6)  len=6  (T_ERROR; stale len=6)
    -> rc=0 iters=4                              <- TERMINATES (success)
(all 8 test inputs terminate in ≤5 iterations; none hit the 100000-iter cap)
```

The stale `len` at the unclosed-quote iteration is **1** (test 0) or **6**
(test 4) — never 0. `off` advances; the loop ends.

## Reachability caveat (independent of the false mechanism)

Even *if* the loop were real, `NGM_ASCII2BINARY` (the only caller chain into
`ng_parse_skip_value`) is reachable **only through a PF_NETGRAPH control
socket**, whose attach (`ngc_attach`, ng_socket.c:182) requires
`caps_priv_check(SYSCAP_RESTRICTEDROOT)` — i.e. **root**. Verified on the
guest: `ngctl list` as the unprivileged `maxx` user returns
`EPERM ("Operation not permitted")`. So the claimed unprivileged DoS surface
does not exist regardless; at most this would be a root→kernel robustness bug.

## Minor real (non-security) issue observed
When the *first* token of the value is an unclosed quote (no bracket/brace),
`ng_parse_skip_value` returns **success** with `*lenp=0` instead of `EINVAL`
(test 7). This is a cosmetic correctness gap (a malformed value is accepted as
empty), not a hang and not security-relevant. Not warranting a fix.diff for
this finding (whose claimed impact — permanent kernel-thread hang — does not
exist).

## Files
- `poc.c` — netgraph NGM_ASCII2BINARY harness (root-only; included for completeness)
- `parsertest.c` — faithful copy of the 3 parser functions; the authoritative algorithm test
- `parse_run.log` — decisive run output showing all inputs terminate
- `env.txt` — guest environment