From 36198800a3e5a58aae6b4b141e5c8752d736e6f9 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Tue, 23 Jun 2026 14:52:48 -0700 Subject: [PATCH] Strip trailing comma from peer name lists - GetNameListRaw folded a trailing comma into the last name, so NameToId returned ID_UNKNOWN and negotiation failed. - Trim one trailing comma up front, matching AlgoListSz. - Add regression test for a KEX list with a trailing comma. Issue: F-2478 --- src/internal.c | 5 +++++ tests/regress.c | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/internal.c b/src/internal.c index d4ebb91a..4f8b1401 100644 --- a/src/internal.c +++ b/src/internal.c @@ -4181,6 +4181,11 @@ static int GetNameListRaw(byte* idList, word32* idListSz, return WS_BAD_ARGUMENT; } + /* Adjust nameListSz for a trailing comma. */ + if (nameListSz > 0 && nameList[nameListSz - 1] == ',') { + nameListSz--; + } + /* 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. */ diff --git a/tests/regress.c b/tests/regress.c index 11cf806d..b0d2086e 100644 --- a/tests/regress.c +++ b/tests/regress.c @@ -2980,6 +2980,40 @@ static void TestKexInitNameListCaps(void) WFREE(list, NULL, DYNTYPE_STRING); } +/* A peer name list may legally end with a trailing comma. GetNameListRaw + * used to fold that comma into the final name, yielding ID_UNKNOWN and a + * failed negotiation. Build a KEXINIT whose KEX list ends in a comma and + * assert the algorithm still negotiates. */ +static void TestKexInitTrailingComma(void) +{ + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + byte payload[512]; + word32 payloadSz; + word32 idx = 0; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); + AssertNotNull(ctx); + ssh = wolfSSH_new(ctx); + AssertNotNull(ssh); + AssertIntEQ(wolfSSH_SetAlgoListKex(ssh, FPF_KEX_GOOD), WS_SUCCESS); + AssertIntEQ(wolfSSH_SetAlgoListKey(ssh, FPF_KEY_GOOD), WS_SUCCESS); + + /* KEX list carries a trailing comma. */ + payloadSz = BuildKexInitPayload(ssh, FPF_KEX_GOOD ",", FPF_KEY_GOOD, + 0, payload, (word32)sizeof(payload)); + + /* The tail (host key/send) errors on this bare ssh, but negotiation + * runs first and records the result in handshake->kexId. */ + (void)wolfSSH_TestDoKexInit(ssh, payload, payloadSz, &idx); + + AssertNotNull(ssh->handshake); + AssertIntEQ(ssh->handshake->kexId, ID_ECDH_SHA2_NISTP256); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +} + #if !defined(WOLFSSH_NO_AES_CBC) && !defined(WOLFSSH_NO_AES_CTR) \ && !defined(WOLFSSH_NO_HMAC_SHA1) && !defined(WOLFSSH_NO_HMAC_SHA2_256) static void TestIndependentAlgoNegotiation(void) @@ -4391,6 +4425,7 @@ int main(int argc, char** argv) TestFirstPacketFollows(); TestKexInitReservedNonZeroRejected(); TestKexInitNameListCaps(); + TestKexInitTrailingComma(); TestDoKexInitRejectsWhenPeerIsKeying(); #endif #if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_RSA) \