Skip to content

Bound KEXINIT name-list parsing to prevent pre-auth CPU DoS#1062

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_5726
Open

Bound KEXINIT name-list parsing to prevent pre-auth CPU DoS#1062
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_5726

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown
Contributor

Summary

GetNameListRaw parsed peer-supplied SSH name-lists with no per-field byte or token-count limit. For every comma-delimited token it called NameToId, which does a linear scan of the ~55-entry NameIdMap (a WSTRLEN + XMEMCMP per entry). Repeated unknown tokens are never stored in idList (only the first is kept), so the existing "no more space" guard never trips and the loop keeps scanning every token.

A single SSH_MSG_KEXINIT can carry name-list fields up to MAX_PACKET_SZ, and DoKexInit parses eight of them — before authentication. A field of a,a,a,… (≈16K single-char tokens in 32 KB) yields O(tokens × map) work per field, measured at ~5.3 ms CPU per max-size KEXINIT. Sustained flooding from parallel connections can saturate a server core without ever completing user auth.

This is a pre-authentication algorithmic-complexity DoS addressed by f_5726.

Fix

Cap each parsed name-list in GetNameListRaw:

  • WOLFSSH_MAX_NAMELIST_SZ (4096) — per-field byte limit, rejected up front.
  • WOLFSSH_MAX_NAMELIST_CNT (64) — per-field name-count limit, checked before each NameToId call.

Either limit exceeded returns WS_BUFFER_E. Both caps are #ifndef-guarded so they remain tunable.

The caps sit well above any legitimate list: NameIdMap has fewer than 64 entries, and the built-in canned algorithm lists are a few hundred bytes — so real handshakes are unaffected. The bound applies to every list parsed through this function (the canned lists are far under the caps).

/* Reject oversized name-lists to bound the per-token NameToId scan cost.
 * Applies to every list parsed here; the built-in canned lists are far
 * under these caps. */
if (nameListSz > WOLFSSH_MAX_NAMELIST_SZ) {
    WLOG(WS_LOG_ERROR, "Name list too large");
    return WS_BUFFER_E;
}
...
if (++tokenCount > WOLFSSH_MAX_NAMELIST_CNT) {
    WLOG(WS_LOG_ERROR, "Too many names in list");
    return WS_BUFFER_E;
}

Tests

Added TestKexInitNameListCaps() in tests/regress.c, driving crafted KEXINIT payloads through the existing wolfSSH_TestDoKexInitGetNameListGetNameListRaw path. The tokens are unknown (a,a,…) so idList never fills for an unrelated reason — the cap is the only thing under test. Both boundaries are locked in:

Case Field Expected
name count at cap 64 names parses, then WS_MATCH_KEX_ALGO_E
name count over cap 65 names WS_BUFFER_E
byte size at cap 4096-byte token parses, then WS_MATCH_KEX_ALGO_E
byte size over cap 4097-byte token WS_BUFFER_E

Verification

  • ./configure --enable-all + full make check (10 suites) — all PASS.
  • Rebuilt with -fsanitize=address,undefined; full make check — all PASS, no ASan/UBSan reports. (macOS: LeakSanitizer unsupported, so leak detection disabled; the new test pairs every WMALLOC with a WFREE on all paths.)
  • Negative control: with the token cap neutralized, the 65-name case returned -1050 (WS_MATCH_KEX_ALGO_E) instead of -1004 (WS_BUFFER_E) and the test failed as expected — confirming it catches the regression.

Files changed

  • wolfssh/internal.h — add WOLFSSH_MAX_NAMELIST_SZ / WOLFSSH_MAX_NAMELIST_CNT.
  • src/internal.c — enforce both caps in GetNameListRaw.
  • tests/regress.c — add TestKexInitNameListCaps() and helpers.

@yosuke-wolfssl yosuke-wolfssl self-assigned this Jun 26, 2026
Copilot AI review requested due to automatic review settings June 26, 2026 04:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #1062

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants