Skip to content

Commit 422f69d

Browse files
yosuke-wolfsslejohnstown
authored andcommitted
Bound KEXINIT name-list parsing to prevent pre-auth CPU DoS
1 parent 8ec9aa7 commit 422f69d

3 files changed

Lines changed: 121 additions & 1 deletion

File tree

src/internal.c

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3908,13 +3908,21 @@ static int GetNameListRaw(byte* idList, word32* idListSz,
39083908
const byte* nameList, word32 nameListSz)
39093909
{
39103910
const byte* name = nameList;
3911-
word32 nameSz = 0, nameListIdx = 0, idListIdx = 0;
3911+
word32 nameSz = 0, nameListIdx = 0, idListIdx = 0, tokenCount = 0;
39123912
int ret = WS_SUCCESS;
39133913

39143914
if (idList == NULL || nameList == NULL || idListSz == NULL) {
39153915
return WS_BAD_ARGUMENT;
39163916
}
39173917

3918+
/* Reject oversized name-lists to bound the per-token NameToId scan cost.
3919+
* Applies to every list parsed here; the built-in canned lists are far
3920+
* under these caps. */
3921+
if (nameListSz > WOLFSSH_MAX_NAMELIST_SZ) {
3922+
WLOG(WS_LOG_ERROR, "Name list too large");
3923+
return WS_BUFFER_E;
3924+
}
3925+
39183926
/*
39193927
* The strings we want are now in the bounds of the message, and the
39203928
* length of the list. Find the commas, or end of list, and then decode
@@ -3930,6 +3938,11 @@ static int GetNameListRaw(byte* idList, word32* idListSz,
39303938
if (nameListIdx == nameListSz || name[nameSz] == ',') {
39313939
byte id;
39323940

3941+
if (++tokenCount > WOLFSSH_MAX_NAMELIST_CNT) {
3942+
WLOG(WS_LOG_ERROR, "Too many names in list");
3943+
return WS_BUFFER_E;
3944+
}
3945+
39333946
id = NameToId((char*)name, nameSz);
39343947
{
39353948
const char* displayName = IdToName(id);

tests/regress.c

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2882,6 +2882,104 @@ static void TestKexInitReservedNonZeroRejected(void)
28822882
wolfSSH_CTX_free(ctx);
28832883
}
28842884

2885+
/* Run a KEXINIT whose KEX-algorithms field is kexList through DoKexInit. The
2886+
* list uses unknown tokens, so on accept it fails to match
2887+
* (WS_MATCH_KEX_ALGO_E); a list rejected by the caps returns WS_BUFFER_E. */
2888+
static int RunKexInitKexListCase(const char* kexList)
2889+
{
2890+
WOLFSSH_CTX* ctx;
2891+
WOLFSSH* ssh;
2892+
byte* payload;
2893+
word32 payloadSz;
2894+
word32 idx = 0;
2895+
int ret;
2896+
word32 bufSz = WOLFSSH_MAX_NAMELIST_SZ + 2048;
2897+
2898+
ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
2899+
AssertNotNull(ctx);
2900+
ssh = wolfSSH_new(ctx);
2901+
AssertNotNull(ssh);
2902+
AssertIntEQ(wolfSSH_SetAlgoListKex(ssh, FPF_KEX_GOOD), WS_SUCCESS);
2903+
AssertIntEQ(wolfSSH_SetAlgoListKey(ssh, FPF_KEY_GOOD), WS_SUCCESS);
2904+
2905+
payload = (byte*)WMALLOC(bufSz, NULL, DYNTYPE_BUFFER);
2906+
AssertNotNull(payload);
2907+
2908+
payloadSz = BuildKexInitPayload(ssh, kexList, FPF_KEY_GOOD, 0,
2909+
payload, bufSz);
2910+
ret = wolfSSH_TestDoKexInit(ssh, payload, payloadSz, &idx);
2911+
2912+
WFREE(payload, NULL, DYNTYPE_BUFFER);
2913+
wolfSSH_free(ssh);
2914+
wolfSSH_CTX_free(ctx);
2915+
2916+
return ret;
2917+
}
2918+
2919+
/* Build "a,a,...,a" with count unknown tokens into a freshly allocated,
2920+
* NULL-terminated buffer. */
2921+
static char* BuildTokenListStr(word32 count)
2922+
{
2923+
char* list;
2924+
word32 listSz;
2925+
word32 i;
2926+
2927+
AssertTrue(count > 0);
2928+
listSz = (count * 2) - 1;
2929+
2930+
list = (char*)WMALLOC(listSz + 1, NULL, DYNTYPE_STRING);
2931+
AssertNotNull(list);
2932+
for (i = 0; i < count; i++) {
2933+
list[i * 2] = 'a';
2934+
if (i + 1 < count)
2935+
list[(i * 2) + 1] = ',';
2936+
}
2937+
list[listSz] = '\0';
2938+
2939+
return list;
2940+
}
2941+
2942+
/* Build a single unknown token of byteSz 'a' characters. */
2943+
static char* BuildSingleTokenStr(word32 byteSz)
2944+
{
2945+
char* list;
2946+
2947+
list = (char*)WMALLOC(byteSz + 1, NULL, DYNTYPE_STRING);
2948+
AssertNotNull(list);
2949+
WMEMSET(list, 'a', byteSz);
2950+
list[byteSz] = '\0';
2951+
2952+
return list;
2953+
}
2954+
2955+
/* Lock in both KEXINIT name-list boundaries (name count and byte size) so a
2956+
* future retune or refactor that disables the bound or flips the off-by-one
2957+
* fails here. */
2958+
static void TestKexInitNameListCaps(void)
2959+
{
2960+
char* list;
2961+
2962+
/* Exactly the name-count cap parses (then fails to match). */
2963+
list = BuildTokenListStr(WOLFSSH_MAX_NAMELIST_CNT);
2964+
AssertIntEQ(RunKexInitKexListCase(list), WS_MATCH_KEX_ALGO_E);
2965+
WFREE(list, NULL, DYNTYPE_STRING);
2966+
2967+
/* One past the name-count cap is rejected. */
2968+
list = BuildTokenListStr(WOLFSSH_MAX_NAMELIST_CNT + 1);
2969+
AssertIntEQ(RunKexInitKexListCase(list), WS_BUFFER_E);
2970+
WFREE(list, NULL, DYNTYPE_STRING);
2971+
2972+
/* Exactly the byte cap parses (then fails to match). */
2973+
list = BuildSingleTokenStr(WOLFSSH_MAX_NAMELIST_SZ);
2974+
AssertIntEQ(RunKexInitKexListCase(list), WS_MATCH_KEX_ALGO_E);
2975+
WFREE(list, NULL, DYNTYPE_STRING);
2976+
2977+
/* One past the byte cap is rejected. */
2978+
list = BuildSingleTokenStr(WOLFSSH_MAX_NAMELIST_SZ + 1);
2979+
AssertIntEQ(RunKexInitKexListCase(list), WS_BUFFER_E);
2980+
WFREE(list, NULL, DYNTYPE_STRING);
2981+
}
2982+
28852983
#if !defined(WOLFSSH_NO_AES_CBC) && !defined(WOLFSSH_NO_AES_CTR) \
28862984
&& !defined(WOLFSSH_NO_HMAC_SHA1) && !defined(WOLFSSH_NO_HMAC_SHA2_256)
28872985
static void TestIndependentAlgoNegotiation(void)
@@ -4292,6 +4390,7 @@ int main(int argc, char** argv)
42924390
&& !defined(WOLFSSH_NO_RSA_SHA2_256)
42934391
TestFirstPacketFollows();
42944392
TestKexInitReservedNonZeroRejected();
4393+
TestKexInitNameListCaps();
42954394
TestDoKexInitRejectsWhenPeerIsKeying();
42964395
#endif
42974396
#if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_RSA) \

wolfssh/internal.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,14 @@ enum NameIdType {
464464
/* This is from RFC 4253 section 6.1. */
465465
#define MAX_PACKET_SZ 35000
466466
#endif
467+
#ifndef WOLFSSH_MAX_NAMELIST_SZ
468+
/* Per-field byte cap on a peer-supplied SSH name-list. */
469+
#define WOLFSSH_MAX_NAMELIST_SZ 4096
470+
#endif
471+
#ifndef WOLFSSH_MAX_NAMELIST_CNT
472+
/* Per-field cap on the number of names in a name-list. */
473+
#define WOLFSSH_MAX_NAMELIST_CNT 64
474+
#endif
467475
#ifndef WOLFSSH_DEFAULT_GEXDH_MIN
468476
#define WOLFSSH_DEFAULT_GEXDH_MIN 1024
469477
#endif

0 commit comments

Comments
 (0)