fix: heap-buffer-overflow READ in wolfSSL_i2c_ASN1_INTEGER via CRL revoked serial#10798
Open
MarkAtwood wants to merge 1 commit into
Open
fix: heap-buffer-overflow READ in wolfSSL_i2c_ASN1_INTEGER via CRL revoked serial#10798MarkAtwood wants to merge 1 commit into
MarkAtwood wants to merge 1 commit into
Conversation
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.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an ASAN-confirmed heap-buffer-overflow read in the OpenSSL-compat CRL revoked-serial handling by ensuring WOLFSSL_ASN1_INTEGER data is formatted as the downstream encoder expects, and by enabling bounds checks during DER length parsing.
Changes:
src/x509.c: Buildrev->serialNumberas DER (tag + length + value) instead of copying raw serial bytes.src/ssl_asn1.c: Enable bounds enforcement inwolfSSL_i2c_ASN1_INTEGER()by callingGetLength_ex(..., check=1).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/x509.c | Prepends DER tag/length to revoked serial number buffer to match wolfSSL_i2c_ASN1_INTEGER() expectations. |
| src/ssl_asn1.c | Turns on defensive bounds checking when decoding DER length in wolfSSL_i2c_ASN1_INTEGER(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+11026
to
+11030
| 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; |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RevokedCertToRevoked()(src/x509.c) copies raw serial content bytes intoserial->dataand setsdataMax = serialSz.wolfSSL_i2c_ASN1_INTEGER()(src/ssl_asn1.c) assumesdata[0]is the DER tag byte anddata[1]is the length byte, then callsGetLength_exwithcheck=0(no bounds enforcement). A crafted CRL with a serial whose second byte is large (e.g.0x7f) causesXMEMCPYto read far past the end of the allocation.Attack path — no signature verification required:
Confirmed with ASAN: READ of size 127 from a 4-byte heap region (
ssl_asn1.c:1711), wolfSSL 5.9.2 HEAD.Fix 1 (root cause) —
src/x509.c: prependASN_INTEGERtag and length byte toserial->data; allocateserialSz + 2and setdataMax = serialSz + 2, matching the layouti2c_ASN1_INTEGERexpects.Fix 2 (defense-in-depth) —
src/ssl_asn1.c: passcheck=1toGetLength_exso bounds are enforced regardless of caller.All 9 smoke-test configs pass (
sanitize-asan,enable-all,opensslextra,default, etc.).