Skip to content

Fix ReqCertFromX509 to check bounds #10181

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
embhorn wants to merge 4 commits into
base: master
Choose a base branch
from
+118 −7
Open

Fix ReqCertFromX509 to check bounds #10181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1bb7c64
Fix wolfSSL_sk_X509_OBJECT_deep_copy to check bounds
embhorn Apr 9, 2026
f98634c
Fix from review
embhorn Apr 9, 2026
8154bc6
Fix from review
embhorn Apr 10, 2026
ddba3cd
Fix from review
embhorn Apr 10, 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
27 changes: 21 additions & 6 deletions src/x509.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11703,23 +11703,38 @@ WOLF_STACK_OF(WOLFSSL_X509_OBJECT)* wolfSSL_sk_X509_OBJECT_deep_copy(
}

if (ret == WOLFSSL_SUCCESS) {
#if defined(OPENSSL_ALL)
int idx;
#endif

cert->version = req->version;
cert->isCA = req->isCa;
cert->basicConstSet = req->basicConstSet;
#ifdef WOLFSSL_CERT_EXT
if (req->subjKeyIdSz != 0) {
XMEMCPY(cert->skid, req->subjKeyId, req->subjKeyIdSz);
cert->skidSz = (int)req->subjKeyIdSz;
if (req->subjKeyIdSz > CTC_MAX_SKID_SIZE) {
WOLFSSL_MSG("Subject Key ID too large");
WOLFSSL_ERROR_VERBOSE(BUFFER_E);
ret = WOLFSSL_FAILURE;
}
else if (req->subjKeyId == NULL) {
WOLFSSL_MSG("Subject Key ID missing");
WOLFSSL_ERROR_VERBOSE(BAD_FUNC_ARG);
ret = WOLFSSL_FAILURE;
}
else {
XMEMCPY(cert->skid, req->subjKeyId,
req->subjKeyIdSz);
cert->skidSz = (int)req->subjKeyIdSz;
}
}
if (req->keyUsageSet)
cert->keyUsage = req->keyUsage;

cert->extKeyUsage = req->extKeyUsage;
#endif
}

if (ret == WOLFSSL_SUCCESS) {
#if defined(OPENSSL_ALL)
int idx;
#endif

XMEMCPY(cert->challengePw, req->challengePw, CTC_NAME_SIZE);
cert->challengePwPrintableString = req->challengePw[0] != 0;
Expand Down
94 changes: 94 additions & 0 deletions tests/api/test_x509.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -875,3 +875,97 @@ int test_x509_CertFromX509_akid_overflow(void)
#endif
return EXPECT_RESULT();
}

/* Test that ReqCertFromX509() rejects a CSR with an oversized
* SubjectKeyIdentifier (> CTC_MAX_SKID_SIZE = 32 bytes). Before the fix
* this would cause a heap buffer overflow into cert->skid[32]. */
int test_x509_ReqCertFromX509_skid_overflow(void)
{
EXPECT_DECLS;
#if defined(WOLFSSL_CERT_REQ) && defined(WOLFSSL_CERT_GEN) && \
defined(WOLFSSL_CERT_EXT) && !defined(NO_BIO) && \
(defined(OPENSSL_EXTRA) || defined(OPENSSL_ALL)) && \
defined(HAVE_ECC)

/* Minimal DER-encoded CSR (PKCS#10) containing a SubjectKeyIdentifier
* extension with a 64-byte value -- double the 32-byte CTC_MAX_SKID_SIZE
* destination buffer.
*
* Structure:
* SEQUENCE {
* CertificationRequestInfo SEQUENCE {
* version INTEGER 0
* subject: CN=Test
* subjectPKInfo: EC P-256 (generator point)
* attributes [0] {
* SEQUENCE {
* OID 1.2.840.113549.1.9.14 (extensionRequest)
* SET { SEQUENCE { -- Extensions
* SEQUENCE { -- Extension
* OID 2.5.29.14 (subjectKeyIdentifier)
* OCTET STRING { OCTET STRING (64 bytes of 0x41) }
* }
* }}
* }
* }
* }
* signatureAlgorithm: ecdsa-with-SHA256
* signatureValue: dummy
* }
*/
static const unsigned char crafted_csr_der[] = {
0x30, 0x81, 0xE7, 0x30, 0x81, 0xCD, 0x02, 0x01,
0x00, 0x30, 0x0F, 0x31, 0x0D, 0x30, 0x0B, 0x06,
0x03, 0x55, 0x04, 0x03, 0x0C, 0x04, 0x54, 0x65,
0x73, 0x74, 0x30, 0x59, 0x30, 0x13, 0x06, 0x07,
0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x02, 0x01, 0x06,
0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01,
0x07, 0x03, 0x42, 0x00, 0x04, 0x6B, 0x17, 0xD1,
0xF2, 0xE1, 0x2C, 0x42, 0x47, 0xF8, 0xBC, 0xE6,
0xE5, 0x63, 0xA4, 0x40, 0xF2, 0x77, 0x03, 0x7D,
0x81, 0x2D, 0xEB, 0x33, 0xA0, 0xF4, 0xA1, 0x39,
0x45, 0xD8, 0x98, 0xC2, 0x96, 0x4F, 0xE3, 0x42,
0xE2, 0xFE, 0x1A, 0x7F, 0x9B, 0x8E, 0xE7, 0xEB,
0x4A, 0x7C, 0x0F, 0x9E, 0x16, 0x2B, 0xCE, 0x33,
0x57, 0x6B, 0x31, 0x5E, 0xCE, 0xCB, 0xB6, 0x40,
0x68, 0x37, 0xBF, 0x51, 0xF5, 0xA0, 0x5C, 0x30,
0x5A, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7,
0x0D, 0x01, 0x09, 0x0E, 0x31, 0x4D, 0x30, 0x4B,
0x30, 0x49, 0x06, 0x03, 0x55, 0x1D, 0x0E, 0x04,
0x42, 0x04, 0x40,
/* 64 bytes of 0x41 -- oversized SKID value */
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
/* end of SKID payload */
0x30, 0x0A, 0x06, 0x08, 0x2A, 0x86, 0x48, 0xCE,
0x3D, 0x04, 0x03, 0x02, 0x03, 0x09, 0x00, 0x30,
0x06, 0x02, 0x01, 0x01, 0x02, 0x01, 0x01
};

WOLFSSL_X509* req = NULL;
WOLFSSL_BIO* bio = NULL;

/* Step 1: Parse the crafted CSR -- this should succeed (the parser
* dynamically allocates subjKeyId to the parsed size). */
req = wolfSSL_X509_REQ_d2i(NULL, crafted_csr_der,
(int)sizeof(crafted_csr_der));
ExpectNotNull(req);

/* Step 2: Attempt to re-encode. Before the fix, this triggered a
* heap buffer overflow in ReqCertFromX509() writing 64 bytes into
* cert->skid[32]. With the fix, it must return failure. */
bio = wolfSSL_BIO_new(wolfSSL_BIO_s_mem());
ExpectNotNull(bio);
ExpectIntNE(wolfSSL_i2d_X509_REQ_bio(bio, req), WOLFSSL_SUCCESS);

wolfSSL_BIO_free(bio);
wolfSSL_X509_free(req);
#endif
return EXPECT_RESULT();
}
4 changes: 3 additions & 1 deletion tests/api/test_x509.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,15 @@ int test_x509_set_serialNumber(void);
int test_x509_verify_cert_hostname_check(void);
int test_x509_time_field_overread_via_tls(void);
int test_x509_CertFromX509_akid_overflow(void);
int test_x509_ReqCertFromX509_skid_overflow(void);

#define TEST_X509_DECLS \
TEST_DECL_GROUP("x509", test_x509_rfc2818_verification_callback), \
TEST_DECL_GROUP("x509", test_x509_GetCAByAKID), \
TEST_DECL_GROUP("x509", test_x509_set_serialNumber), \
TEST_DECL_GROUP("x509", test_x509_verify_cert_hostname_check), \
TEST_DECL_GROUP("x509", test_x509_time_field_overread_via_tls), \
TEST_DECL_GROUP("x509", test_x509_CertFromX509_akid_overflow)
TEST_DECL_GROUP("x509", test_x509_CertFromX509_akid_overflow), \
TEST_DECL_GROUP("x509", test_x509_ReqCertFromX509_skid_overflow)

#endif /* WOLFCRYPT_TEST_X509_H */
Loading