Skip to content

Various fixes #10142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kareem-wolfssl wants to merge 7 commits into
base: master
Choose a base branch
from
+44 −4
Open

Various fixes #10142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1cd5851
Ensure the SNI extension has at least OPAQUE16_LEN bytes in TLSX_SNI_…
kareem-wolfssl Apr 6, 2026
257cae0
Prevent cert chain count from exceeding array max size when calling W…
kareem-wolfssl Apr 6, 2026
e815571
Ensure ProcessChainOCSPRequest does not exceed the length of the cert…
kareem-wolfssl Apr 6, 2026
9254bf7
Enforce max size of responses array in SendCertificateStatus.
kareem-wolfssl Apr 6, 2026
d8fb381
In SSL sniffer, ensure the ClientHello extension length is sufficient…
kareem-wolfssl Apr 6, 2026
392f7e2
Code review feedback
kareem-wolfssl Apr 6, 2026
d83ef81
Fix incorrect logging function name.
kareem-wolfssl Apr 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25909,6 +25909,10 @@ int SendCertificateStatus(WOLFSSL* ssl)

if (idx > chain->length)
break;
if ((i + 1) >= MAX_CERT_EXTENSIONS) {
ret = MAX_CERT_EXTENSIONS_ERR;
break;
}
ret = CreateOcspRequest(ssl, request, cert, der.buffer,
der.length, &ctxOwnsRequest);
if (ret == 0) {
Expand Down Expand Up @@ -25937,6 +25941,11 @@ int SendCertificateStatus(WOLFSSL* ssl)
else {
while (ret == 0 &&
NULL != (request = ssl->ctx->chainOcspRequest[i])) {
if ((i + 1) >= MAX_CERT_EXTENSIONS) {
ret = MAX_CERT_EXTENSIONS_ERR;
break;
}

request->ssl = ssl;
ret = CheckOcspRequest(SSL_CM(ssl)->ocsp_stapling,
request, &responses[++i], ssl->heap);
Expand Down
16 changes: 15 additions & 1 deletion src/sniffer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4195,6 +4195,9 @@ static int ProcessClientHello(const byte* input, int* sslBytes,
{
word16 listLen = 0, offset = 0;

if (extLen < OPAQUE16_LEN)
return BUFFER_ERROR;
Comment on lines +4198 to +4199
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

In this parsing path the function returns BUFFER_ERROR without calling SetError(...), while the other newly added length checks in this function set the error state first. For consistent diagnostics and state handling, consider calling SetError(BUFFER_ERROR_STR, error, session, FATAL_ERROR_STATE) here as well (or use the same non-fatal/fatal pattern used elsewhere in this function).

Copilot uses AI. Check for mistakes.

ato16(input + offset, &listLen);
offset += OPAQUE16_LEN;

Expand Down Expand Up @@ -4228,7 +4231,13 @@ static int ProcessClientHello(const byte* input, int* sslBytes,
#ifdef WOLFSSL_TLS13
case EXT_KEY_SHARE:
{
word16 ksLen = (word16)((input[0] << 8) | input[1]);
word16 ksLen = 0;
if (extLen < OPAQUE16_LEN) {
SetError(BUFFER_ERROR_STR, error, session, FATAL_ERROR_STATE);
return BUFFER_ERROR;
}

ksLen = (word16)((input[0] << 8) | input[1]);
if (ksLen + OPAQUE16_LEN > extLen) {
SetError(CLIENT_HELLO_INPUT_STR, error, session, FATAL_ERROR_STATE);
return WOLFSSL_FATAL_ERROR;
Expand All @@ -4252,6 +4261,11 @@ static int ProcessClientHello(const byte* input, int* sslBytes,
word32 ticketAge;
const byte *identity, *binders;

if (extLen < OPAQUE16_LEN) {
SetError(BUFFER_ERROR_STR, error, session, FATAL_ERROR_STATE);
return BUFFER_ERROR;
}

idsLen = (word16)((input[idx] << 8) | input[idx+1]);
if ((word32)idsLen + OPAQUE16_LEN + idx > (word32)extLen) {
SetError(CLIENT_HELLO_INPUT_STR, error, session, FATAL_ERROR_STATE);
Comment on lines +4264 to 4271
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

The new guard checks only extLen < OPAQUE16_LEN, but idsLen is read from input[idx]/input[idx+1]. If idx is non-zero (or near the end), this can still read out of bounds before the later size validation. Update the pre-check to ensure idx + OPAQUE16_LEN <= extLen (and fail before reading input[idx+1]).

Copilot uses AI. Check for mistakes.
Expand Down
11 changes: 11 additions & 0 deletions src/tls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2800,6 +2800,9 @@ int TLSX_SNI_GetFromBuffer(const byte* clientHello, word32 helloSz,
} else {
word16 listLen;

if (extLen < OPAQUE16_LEN)
return BUFFER_ERROR;

ato16(clientHello + offset, &listLen);
offset += OPAQUE16_LEN;

Expand Down Expand Up @@ -3611,6 +3614,14 @@ int ProcessChainOCSPRequest(WOLFSSL* ssl)

if (chain && chain->buffer) {
while (ret == 0 && pos + OPAQUE24_LEN < chain->length) {
if (i >= MAX_CERT_EXTENSIONS) {
WOLFSSL_MSG_EX(
"OCSP request cert chain exceeds maximum length: "
"i=%d, MAX_CERT_EXTENSIONS=%d", i, MAX_CERT_EXTENSIONS);
ret = MAX_CERT_EXTENSIONS_ERR;
break;
}

c24to32(chain->buffer + pos, &der.length);
pos += OPAQUE24_LEN;
der.buffer = chain->buffer + pos;
Expand Down
12 changes: 9 additions & 3 deletions src/tls13.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8871,7 +8871,7 @@ static word32 NextCert(byte* data, word32 length, word32* idx)
* extIdx The index number of certificate status request data
* for the certificate.
* offset index offset
* returns Total number of bytes written.
* returns Total number of bytes written on success or negative value on error.
*/
static int WriteCSRToBuffer(WOLFSSL* ssl, DerBuffer** certExts,
word16* extSz, word16 extSz_num)
Expand All @@ -8886,6 +8886,9 @@ static int WriteCSRToBuffer(WOLFSSL* ssl, DerBuffer** certExts,
word32 extIdx;
DerBuffer* der;

if (extSz_num > MAX_CERT_EXTENSIONS)
return MAX_CERT_EXTENSIONS_ERR;

ext = TLSX_Find(ssl->extensions, TLSX_STATUS_REQUEST);
csr = ext ? (CertificateStatusRequest*)ext->data : NULL;

Expand Down Expand Up @@ -9137,8 +9140,11 @@ static int SendTls13Certificate(WOLFSSL* ssl)
if (ret != 0)
return ret;

ret = WriteCSRToBuffer(ssl, &ssl->buffers.certExts[0], &extSz[0],
1 /* +1 for leaf */ + (word16)ssl->buffers.certChainCnt);
if ((1 + ssl->buffers.certChainCnt) > MAX_CERT_EXTENSIONS)
ret = MAX_CERT_EXTENSIONS_ERR;
if (ret == 0)
ret = WriteCSRToBuffer(ssl, &ssl->buffers.certExts[0], &extSz[0],
1 /* +1 for leaf */ + (word16)ssl->buffers.certChainCnt);
if (ret < 0)
return ret;
totalextSz += ret;
Expand Down
Loading