Skip to content

Harden chain depth bounds and parser input validation #10209

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

Draft
ColtonWilley wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Harden chain depth bounds and parser input validation #10209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25913,7 +25913,8 @@ int SendCertificateStatus(WOLFSSL* ssl)
}

if (chain && chain->buffer) {
while (ret == 0 && idx + OPAQUE24_LEN < chain->length) {
while (ret == 0 && i < MAX_CHAIN_DEPTH &&
idx + OPAQUE24_LEN < chain->length) {
c24to32(chain->buffer + idx, &der.length);
idx += OPAQUE24_LEN;

Expand Down Expand Up @@ -25948,7 +25949,7 @@ int SendCertificateStatus(WOLFSSL* ssl)
WC_FREE_VAR_EX(cert, ssl->heap, DYNAMIC_TYPE_DCERT);
}
else {
while (ret == 0 &&
while (ret == 0 && i < MAX_CHAIN_DEPTH &&
NULL != (request = ssl->ctx->chainOcspRequest[i])) {
request->ssl = ssl;
ret = CheckOcspRequest(SSL_CM(ssl)->ocsp_stapling,
Expand Down
6 changes: 3 additions & 3 deletions src/ssl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13952,7 +13952,7 @@ int wolfSSL_get_chain_count(WOLFSSL_X509_CHAIN* chain)
int wolfSSL_get_chain_length(WOLFSSL_X509_CHAIN* chain, int idx)
{
WOLFSSL_ENTER("wolfSSL_get_chain_length");
if (chain)
if (chain && idx >= 0 && idx < chain->count)
return chain->certs[idx].length;

return 0;
Expand All @@ -13963,7 +13963,7 @@ int wolfSSL_get_chain_length(WOLFSSL_X509_CHAIN* chain, int idx)
byte* wolfSSL_get_chain_cert(WOLFSSL_X509_CHAIN* chain, int idx)
{
WOLFSSL_ENTER("wolfSSL_get_chain_cert");
if (chain)
if (chain && idx >= 0 && idx < chain->count)
return chain->certs[idx].buffer;

return 0;
Expand All @@ -13978,7 +13978,7 @@ WOLFSSL_X509* wolfSSL_get_chain_X509(WOLFSSL_X509_CHAIN* chain, int idx)
WC_DECLARE_VAR(cert, DecodedCert, 1, 0);

WOLFSSL_ENTER("wolfSSL_get_chain_X509");
if (chain != NULL && idx < MAX_CHAIN_DEPTH) {
if (chain != NULL && idx >= 0 && idx < chain->count) {
#ifdef WOLFSSL_SMALL_STACK
cert = (DecodedCert*)XMALLOC(sizeof(DecodedCert), NULL,
DYNAMIC_TYPE_DCERT);
Expand Down
7 changes: 7 additions & 0 deletions src/ssl_load.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,13 @@ static int ProcessUserChain(WOLFSSL_CTX* ctx, WOLFSSL* ssl,
while ((ret == 0) && (consumed < sz)) {
DerBuffer* part = NULL;

/* Enforce maximum chain depth. */
if (cnt >= MAX_CHAIN_DEPTH) {
WOLFSSL_MSG("Chain depth limit reached");
ret = MAX_CHAIN_ERROR;
break;
}

/* Get a certificate as DER. */
ret = DataToDerBuffer(buff + consumed, (word32)(sz - consumed),
format, type, info, heap, &part, NULL);
Expand Down
14 changes: 12 additions & 2 deletions src/tls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2800,9 +2800,15 @@ 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;

if (listLen != extLen - OPAQUE16_LEN)
return BUFFER_ERROR;

if (helloSz < offset + listLen)
return BUFFER_ERROR;

Expand All @@ -2813,6 +2819,9 @@ int TLSX_SNI_GetFromBuffer(const byte* clientHello, word32 helloSz,
ato16(clientHello + offset, &sniLen);
offset += OPAQUE16_LEN;

if (sniLen > listLen - (ENUM_LEN + OPAQUE16_LEN))
return BUFFER_ERROR;

if (helloSz < offset + sniLen)
return BUFFER_ERROR;

Expand Down Expand Up @@ -3366,7 +3375,7 @@ static void TLSX_CSR_Free(CertificateStatusRequest* csr, void* heap)

switch (csr->status_type) {
case WOLFSSL_CSR_OCSP:
for (i = 0; i <= csr->requests; i++) {
for (i = 0; i < csr->requests; i++) {
FreeOcspRequest(&csr->request.ocsp[i]);
}
break;
Expand Down Expand Up @@ -3610,7 +3619,8 @@ int ProcessChainOCSPRequest(WOLFSSL* ssl)
}

if (chain && chain->buffer) {
while (ret == 0 && pos + OPAQUE24_LEN < chain->length) {
while (ret == 0 && i <= MAX_CHAIN_DEPTH &&
pos + OPAQUE24_LEN < chain->length) {
c24to32(chain->buffer + pos, &der.length);
pos += OPAQUE24_LEN;
der.buffer = chain->buffer + pos;
Expand Down
4 changes: 4 additions & 0 deletions src/tls13.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9132,6 +9132,10 @@ static int SendTls13Certificate(WOLFSSL* ssl)
}
/* Certificate Data */
certSz = ssl->buffers.certificate->length;
if (ssl->buffers.certChainCnt > MAX_CHAIN_DEPTH) {
WOLFSSL_MSG("Certificate chain count exceeds maximum depth");
return MAX_CHAIN_ERROR;
}
/* Cert Req Ctx Len | Cert Req Ctx | Cert List Len | Cert Data Len */
headerSz = OPAQUE8_LEN + certReqCtxLen + CERT_HEADER_SZ +
CERT_HEADER_SZ;
Expand Down
34 changes: 32 additions & 2 deletions wolfcrypt/src/pkcs7.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7032,6 +7032,9 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf,

idx += (word32)length;
}
else if (ret == 0) {
ret = ASN_PARSE_E;
}

pkcs7->content = content;
pkcs7->contentSz = (word32)contentSz;
Expand Down Expand Up @@ -9609,7 +9612,7 @@ static int wc_PKCS7_PwriKek_KeyUnWrap(wc_PKCS7* pkcs7, const byte* kek,
cekLen = outTmp[0];

/* verify length */
fail |= ctMaskGT(cekLen, (int)inSz);
fail |= ctMaskGT(cekLen, (int)inSz - 4);
/* verify check bytes */
fail |= ctMaskNotEq((int)(outTmp[1] ^ outTmp[4]), 0xFF);
fail |= ctMaskNotEq((int)(outTmp[2] ^ outTmp[5]), 0xFF);
Expand Down Expand Up @@ -13021,6 +13024,14 @@ int wc_PKCS7_DecodeEnvelopedData(wc_PKCS7* pkcs7, byte* in,
ret = ASN_PARSE_E;
}

if (ret == 0 && encryptedContentTotalSz > (int)(pkiMsgSz - idx)) {
/* In non-streaming mode, ensure the content fits in the buffer.
* Streaming mode handles this via AddDataToStream. */
#ifdef NO_PKCS7_STREAM
ret = BUFFER_E;
#endif
}

if (ret != 0)
break;

Expand Down Expand Up @@ -14301,6 +14312,12 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in,
}
}

if (ret == 0 && encryptedContentSz > (int)(pkiMsgSz - idx)) {
#ifdef NO_PKCS7_STREAM
ret = BUFFER_E;
#endif
}

if (ret < 0)
break;

Expand Down Expand Up @@ -14546,6 +14563,12 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in,
#endif
idx = localIdx;

#ifdef NO_PKCS7_STREAM
if (ret == 0 && authTagSz > (word32)(pkiMsgSz - idx)) {
ret = BUFFER_E;
}
#endif

if (ret == 0 && authTagSz > (word32)sizeof(authTag)) {
WOLFSSL_MSG("AuthEnvelopedData authTag too large for buffer");
ret = ASN_PARSE_E;
Expand Down Expand Up @@ -15262,6 +15285,12 @@ int wc_PKCS7_DecodeEncryptedData(wc_PKCS7* pkcs7, byte* in, word32 inSz,
pkiMsgSz, NO_USER_CHECK) <= 0)
ret = ASN_PARSE_E;

#ifdef NO_PKCS7_STREAM
if (ret == 0 && encryptedContentSz > (int)(pkiMsgSz - idx)) {
ret = BUFFER_E;
}
#endif

if (ret < 0)
break;
#ifndef NO_PKCS7_STREAM
Expand Down Expand Up @@ -15299,7 +15328,8 @@ int wc_PKCS7_DecodeEncryptedData(wc_PKCS7* pkcs7, byte* in, word32 inSz,
version = (int)pkcs7->stream->vers;
tmpIv = pkcs7->stream->tmpIv;
#endif
if (encryptedContentSz <= 0) {
if (encryptedContentSz <= 0 ||
encryptedContentSz > (int)(pkiMsgSz - idx)) {
ret = BUFFER_E;
break;
}
Expand Down
Loading