Skip to content

Commit af329b3

Browse files
committed
Fix heap buffer over-read in wolfSSL_select_next_proto
Add missing bounds validation in wolfSSL_select_next_proto. Three issues fixed: 1. Outer loop: no check that length byte + position stays within inLen, allowing XMEMCMP to read past the server protocol list buffer. 2. Inner loop: same missing check for clientNames/clientLen boundary. 3. No-overlap fallback unconditionally dereferences clientNames[0] even when clientLen is 0, and returns an outLen that may exceed the buffer. Also reject zero-length protocol entries (invalid per RFC 7301) to prevent infinite loops. Add unit test test_wolfSSL_select_next_proto with 8 cases covering NULL params, normal match, no overlap, malformed length overruns, zero-length entries, and empty client lists.
1 parent 558ae34 commit af329b3

2 files changed

Lines changed: 131 additions & 2 deletions

File tree

src/ssl.c

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16662,8 +16662,12 @@ int wolfSSL_select_next_proto(unsigned char **out, unsigned char *outLen,
1666216662

1666316663
for (i = 0; i < inLen; i += lenIn) {
1666416664
lenIn = in[i++];
16665+
if (lenIn == 0 || i + lenIn > inLen)
16666+
break;
1666516667
for (j = 0; j < clientLen; j += lenClient) {
1666616668
lenClient = clientNames[j++];
16669+
if (lenClient == 0 || j + lenClient > clientLen)
16670+
break;
1666716671

1666816672
if (lenIn != lenClient)
1666916673
continue;
@@ -16676,8 +16680,14 @@ int wolfSSL_select_next_proto(unsigned char **out, unsigned char *outLen,
1667616680
}
1667716681
}
1667816682

16679-
*out = (unsigned char *)clientNames + 1;
16680-
*outLen = clientNames[0];
16683+
if (clientLen > 0 && (unsigned int)clientNames[0] + 1 <= clientLen) {
16684+
*out = (unsigned char *)clientNames + 1;
16685+
*outLen = clientNames[0];
16686+
}
16687+
else {
16688+
*out = (unsigned char *)clientNames;
16689+
*outLen = 0;
16690+
}
1668116691
return WOLFSSL_NPN_NO_OVERLAP;
1668216692
}
1668316693

tests/api.c

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9865,6 +9865,124 @@ static int test_wolfSSL_set_alpn_protos(void)
98659865
return res;
98669866
}
98679867

9868+
static int test_wolfSSL_select_next_proto(void)
9869+
{
9870+
EXPECT_DECLS;
9871+
unsigned char *out = NULL;
9872+
unsigned char outLen = 0;
9873+
int ret;
9874+
9875+
/* Wire format: length-prefixed protocol names */
9876+
unsigned char serverList[] = {
9877+
8, 'h','t','t','p','/','1','.','1',
9878+
6, 's','p','d','y','/','2'
9879+
};
9880+
unsigned int serverLen = sizeof(serverList);
9881+
9882+
unsigned char clientList[] = {
9883+
6, 's','p','d','y','/','2'
9884+
};
9885+
unsigned int clientLen = sizeof(clientList);
9886+
9887+
unsigned char clientListHttp[] = {
9888+
8, 'h','t','t','p','/','1','.','1'
9889+
};
9890+
unsigned int clientListHttpLen = sizeof(clientListHttp);
9891+
9892+
unsigned char clientListNoMatch[] = {
9893+
6, 's','p','d','y','/','3'
9894+
};
9895+
unsigned int clientListNoMatchLen = sizeof(clientListNoMatch);
9896+
9897+
/* Test 1: NULL parameters return UNSUPPORTED */
9898+
ExpectIntEQ(wolfSSL_select_next_proto(NULL, &outLen, serverList,
9899+
serverLen, clientList, clientLen), WOLFSSL_NPN_UNSUPPORTED);
9900+
ExpectIntEQ(wolfSSL_select_next_proto(&out, NULL, serverList,
9901+
serverLen, clientList, clientLen), WOLFSSL_NPN_UNSUPPORTED);
9902+
ExpectIntEQ(wolfSSL_select_next_proto(&out, &outLen, NULL,
9903+
serverLen, clientList, clientLen), WOLFSSL_NPN_UNSUPPORTED);
9904+
ExpectIntEQ(wolfSSL_select_next_proto(&out, &outLen, serverList,
9905+
serverLen, NULL, clientLen), WOLFSSL_NPN_UNSUPPORTED);
9906+
9907+
/* Test 2: Normal match - client wants "spdy/2", server offers it */
9908+
out = NULL;
9909+
outLen = 0;
9910+
ret = wolfSSL_select_next_proto(&out, &outLen, serverList, serverLen,
9911+
clientList, clientLen);
9912+
ExpectIntEQ(ret, WOLFSSL_NPN_NEGOTIATED);
9913+
ExpectIntEQ(outLen, 6);
9914+
ExpectNotNull(out);
9915+
ExpectIntEQ(XMEMCMP(out, "spdy/2", 6), 0);
9916+
9917+
/* Test 3: No overlap - server offers "http/1.1,spdy/2", client wants
9918+
* "spdy/3". Falls back to first client protocol. */
9919+
out = NULL;
9920+
outLen = 0;
9921+
ret = wolfSSL_select_next_proto(&out, &outLen, serverList, serverLen,
9922+
clientListNoMatch, clientListNoMatchLen);
9923+
ExpectIntEQ(ret, WOLFSSL_NPN_NO_OVERLAP);
9924+
ExpectIntEQ(outLen, 6);
9925+
ExpectNotNull(out);
9926+
ExpectIntEQ(XMEMCMP(out, "spdy/3", 6), 0);
9927+
9928+
/* Test 4: Malformed server list - length byte overruns buffer.
9929+
* Must NOT crash (heap over-read). */
9930+
{
9931+
unsigned char malformedServer[] = { 200, 'h','t','t','p' };
9932+
out = NULL;
9933+
outLen = 0;
9934+
ret = wolfSSL_select_next_proto(&out, &outLen, malformedServer,
9935+
sizeof(malformedServer), clientList, clientLen);
9936+
ExpectIntEQ(ret, WOLFSSL_NPN_NO_OVERLAP);
9937+
}
9938+
9939+
/* Test 5: Malformed client list - length byte overruns buffer.
9940+
* Must NOT crash. */
9941+
{
9942+
unsigned char malformedClient[] = { 200, 's','p','d','y' };
9943+
out = NULL;
9944+
outLen = 0;
9945+
ret = wolfSSL_select_next_proto(&out, &outLen, serverList, serverLen,
9946+
malformedClient, sizeof(malformedClient));
9947+
ExpectIntEQ(ret, WOLFSSL_NPN_NO_OVERLAP);
9948+
}
9949+
9950+
/* Test 6: Zero-length entry in server list - must NOT infinite loop */
9951+
{
9952+
unsigned char zeroLenServer[] = { 0, 6, 's','p','d','y','/','2' };
9953+
out = NULL;
9954+
outLen = 0;
9955+
ret = wolfSSL_select_next_proto(&out, &outLen, zeroLenServer,
9956+
sizeof(zeroLenServer), clientList, clientLen);
9957+
/* Zero-length entry causes break, so no match found */
9958+
ExpectIntEQ(ret, WOLFSSL_NPN_NO_OVERLAP);
9959+
}
9960+
9961+
/* Test 7: Empty client list (clientLen == 0) - must NOT dereference
9962+
* clientNames[0]. */
9963+
{
9964+
unsigned char emptyClient[] = { 0 };
9965+
out = NULL;
9966+
outLen = 0;
9967+
ret = wolfSSL_select_next_proto(&out, &outLen, serverList, serverLen,
9968+
emptyClient, 0);
9969+
ExpectIntEQ(ret, WOLFSSL_NPN_NO_OVERLAP);
9970+
ExpectIntEQ(outLen, 0);
9971+
}
9972+
9973+
/* Test 8: First protocol match - both start with "http/1.1" */
9974+
out = NULL;
9975+
outLen = 0;
9976+
ret = wolfSSL_select_next_proto(&out, &outLen, serverList, serverLen,
9977+
clientListHttp, clientListHttpLen);
9978+
ExpectIntEQ(ret, WOLFSSL_NPN_NEGOTIATED);
9979+
ExpectIntEQ(outLen, 8);
9980+
ExpectNotNull(out);
9981+
ExpectIntEQ(XMEMCMP(out, "http/1.1", 8), 0);
9982+
9983+
return EXPECT_RESULT();
9984+
}
9985+
98689986
#endif /* HAVE_ALPN_PROTOS_SUPPORT */
98699987

98709988
static int test_wolfSSL_wolfSSL_UseSecureRenegotiation(void)
@@ -32842,6 +32960,7 @@ TEST_CASE testCases[] = {
3284232960
#ifdef HAVE_ALPN_PROTOS_SUPPORT
3284332961
/* Uses Assert in handshake callback. */
3284432962
TEST_DECL(test_wolfSSL_set_alpn_protos),
32963+
TEST_DECL(test_wolfSSL_select_next_proto),
3284532964
#endif
3284632965
TEST_DECL(test_tls_ems_downgrade),
3284732966
TEST_DECL(test_wolfSSL_DisableExtendedMasterSecret),

0 commit comments

Comments
 (0)