diff --git a/src/x509.c b/src/x509.c index 46dfd38ed4..6681d00b2a 100644 --- a/src/x509.c +++ b/src/x509.c @@ -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; diff --git a/tests/api/test_x509.c b/tests/api/test_x509.c index 47780e6dc4..1f54510f57 100644 --- a/tests/api/test_x509.c +++ b/tests/api/test_x509.c @@ -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(); +} diff --git a/tests/api/test_x509.h b/tests/api/test_x509.h index adbc980f8b..ffeae91e50 100644 --- a/tests/api/test_x509.h +++ b/tests/api/test_x509.h @@ -28,6 +28,7 @@ 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), \ @@ -35,6 +36,7 @@ int test_x509_CertFromX509_akid_overflow(void); 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 */