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:

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:

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