From c8bb312b69bd5b246937700c6895e64211991af1 Mon Sep 17 00:00:00 2001 From: Mark Atwood Date: Fri, 26 Jun 2026 18:57:15 -0700 Subject: [PATCH] fix: CRL revoked serial heap-buffer-overflow in i2c_ASN1_INTEGER RevokedCertToRevoked() stored raw serial content bytes in serial->data with dataMax=serialSz, but wolfSSL_i2c_ASN1_INTEGER assumes data[0] is the DER tag byte and data[1] is the length byte. GetLength_ex with check=0 then used data[1] as a length without bounds enforcement, allowing an attacker-controlled CRL to trigger an OOB heap read via X509_CRL_get_REVOKED + i2c_ASN1_INTEGER. Fix 1 (root cause): prepend ASN_INTEGER tag and serialSz length byte to serial->data in RevokedCertToRevoked; allocate serialSz+2 bytes and set dataMax=serialSz+2 to match the expected layout. Fix 2 (defense-in-depth): pass check=1 to GetLength_ex in wolfSSL_i2c_ASN1_INTEGER so bounds are enforced even for callers that pass a malformed ASN1_INTEGER. Confirmed via ASAN PoC: READ of size 127 from 4-byte region no longer fires after this patch. --- src/ssl_asn1.c | 2 +- src/x509.c | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/ssl_asn1.c b/src/ssl_asn1.c index 2b7c25f01b5..c999b3d512e 100644 --- a/src/ssl_asn1.c +++ b/src/ssl_asn1.c @@ -1689,7 +1689,7 @@ int wolfSSL_i2c_ASN1_INTEGER(WOLFSSL_ASN1_INTEGER *a, unsigned char **pp) } /* Get length from DER encoding. */ - if ((!err) && (GetLength_ex(a->data, &idx, &len, a->dataMax, 0) < 0)) { + if ((!err) && (GetLength_ex(a->data, &idx, &len, a->dataMax, 1) < 0)) { err = 1; } diff --git a/src/x509.c b/src/x509.c index fd87b1da4aa..8e5ed7d0255 100644 --- a/src/x509.c +++ b/src/x509.c @@ -11013,16 +11013,21 @@ static WOLFSSL_X509_REVOKED* RevokedCertToRevoked(RevokedCert* rc, int seq) return NULL; } if (rc->serialSz > 0 && rc->serialSz <= EXTERNAL_SERIAL_SIZE) { - serial->data = (unsigned char*)XMALLOC((size_t)rc->serialSz, NULL, + /* Allocate tag byte + length byte + content so that + * i2c_ASN1_INTEGER can read data[0] as tag and data[1] as length + * without overrunning the allocation. */ + serial->data = (unsigned char*)XMALLOC((size_t)rc->serialSz + 2, NULL, DYNAMIC_TYPE_OPENSSL); if (serial->data == NULL) { wolfSSL_ASN1_INTEGER_free(serial); XFREE(rev, NULL, DYNAMIC_TYPE_OPENSSL); return NULL; } - XMEMCPY(serial->data, rc->serialNumber, (size_t)rc->serialSz); + serial->data[0] = ASN_INTEGER; + serial->data[1] = (unsigned char)rc->serialSz; + XMEMCPY(serial->data + 2, rc->serialNumber, (size_t)rc->serialSz); serial->length = rc->serialSz; - serial->dataMax = rc->serialSz; + serial->dataMax = (word32)rc->serialSz + 2; serial->isDynamic = 1; } rev->serialNumber = serial;