Skip to content

Commit 46662ba

Browse files
embhornembhorn
committed
Fix wolfSSL_sk_X509_OBJECT_deep_copy to check bounds
1 parent 178e10e commit 46662ba

4 files changed

Lines changed: 111 additions & 5 deletions

File tree

‎src/x509.c‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11712,8 +11712,18 @@ WOLF_STACK_OF(WOLFSSL_X509_OBJECT)* wolfSSL_sk_X509_OBJECT_deep_copy(
1171211712
cert->basicConstSet = req->basicConstSet;
1171311713
#ifdef WOLFSSL_CERT_EXT
1171411714
if (req->subjKeyIdSz != 0) {
11715-
XMEMCPY(cert->skid, req->subjKeyId, req->subjKeyIdSz);
11716-
cert->skidSz = (int)req->subjKeyIdSz;
11715+
if (req->subjKeyIdSz <= CTC_MAX_SKID_SIZE) {
11716+
if (req->subjKeyId) {
11717+
XMEMCPY(cert->skid, req->subjKeyId,
11718+
req->subjKeyIdSz);
11719+
}
11720+
cert->skidSz = (int)req->subjKeyIdSz;
11721+
}
11722+
else {
11723+
WOLFSSL_MSG("Subject Key ID too large");
11724+
WOLFSSL_ERROR_VERBOSE(BUFFER_E);
11725+
ret = WOLFSSL_FAILURE;
11726+
}
1171711727
}
1171811728
if (req->keyUsageSet)
1171911729
cert->keyUsage = req->keyUsage;

‎tests/api/test_x509.c‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,3 +878,97 @@ int test_x509_CertFromX509_akid_overflow(void)
878878
#endif
879879
return EXPECT_RESULT();
880880
}
881+
882+
/* Test that ReqCertFromX509() rejects a CSR with an oversized
883+
* SubjectKeyIdentifier (> CTC_MAX_SKID_SIZE = 32 bytes). Before the fix
884+
* this would cause a heap buffer overflow into cert->skid[32]. */
885+
int test_x509_ReqCertFromX509_skid_overflow(void)
886+
{
887+
EXPECT_DECLS;
888+
#if defined(WOLFSSL_CERT_REQ) && defined(WOLFSSL_CERT_GEN) && \
889+
defined(WOLFSSL_CERT_EXT) && !defined(NO_BIO) && \
890+
(defined(OPENSSL_EXTRA) || defined(OPENSSL_ALL)) && \
891+
defined(HAVE_ECC)
892+
893+
/* Minimal DER-encoded CSR (PKCS#10) containing a SubjectKeyIdentifier
894+
* extension with a 64-byte value — double the 32-byte CTC_MAX_SKID_SIZE
895+
* destination buffer.
896+
*
897+
* Structure:
898+
* SEQUENCE {
899+
* CertificationRequestInfo SEQUENCE {
900+
* version INTEGER 0
901+
* subject: CN=Test
902+
* subjectPKInfo: EC P-256 (generator point)
903+
* attributes [0] {
904+
* SEQUENCE {
905+
* OID 1.2.840.113549.1.9.14 (extensionRequest)
906+
* SET { SEQUENCE { -- Extensions
907+
* SEQUENCE { -- Extension
908+
* OID 2.5.29.14 (subjectKeyIdentifier)
909+
* OCTET STRING { OCTET STRING (64 bytes of 0x41) }
910+
* }
911+
* }}
912+
* }
913+
* }
914+
* }
915+
* signatureAlgorithm: ecdsa-with-SHA256
916+
* signatureValue: dummy
917+
* }
918+
*/
919+
static const unsigned char crafted_csr_der[] = {
920+
0x30, 0x81, 0xE7, 0x30, 0x81, 0xCD, 0x02, 0x01,
921+
0x00, 0x30, 0x0F, 0x31, 0x0D, 0x30, 0x0B, 0x06,
922+
0x03, 0x55, 0x04, 0x03, 0x0C, 0x04, 0x54, 0x65,
923+
0x73, 0x74, 0x30, 0x59, 0x30, 0x13, 0x06, 0x07,
924+
0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x02, 0x01, 0x06,
925+
0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01,
926+
0x07, 0x03, 0x42, 0x00, 0x04, 0x6B, 0x17, 0xD1,
927+
0xF2, 0xE1, 0x2C, 0x42, 0x47, 0xF8, 0xBC, 0xE6,
928+
0xE5, 0x63, 0xA4, 0x40, 0xF2, 0x77, 0x03, 0x7D,
929+
0x81, 0x2D, 0xEB, 0x33, 0xA0, 0xF4, 0xA1, 0x39,
930+
0x45, 0xD8, 0x98, 0xC2, 0x96, 0x4F, 0xE3, 0x42,
931+
0xE2, 0xFE, 0x1A, 0x7F, 0x9B, 0x8E, 0xE7, 0xEB,
932+
0x4A, 0x7C, 0x0F, 0x9E, 0x16, 0x2B, 0xCE, 0x33,
933+
0x57, 0x6B, 0x31, 0x5E, 0xCE, 0xCB, 0xB6, 0x40,
934+
0x68, 0x37, 0xBF, 0x51, 0xF5, 0xA0, 0x5C, 0x30,
935+
0x5A, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7,
936+
0x0D, 0x01, 0x09, 0x0E, 0x31, 0x4D, 0x30, 0x4B,
937+
0x30, 0x49, 0x06, 0x03, 0x55, 0x1D, 0x0E, 0x04,
938+
0x42, 0x04, 0x40,
939+
/* 64 bytes of 0x41 — oversized SKID value */
940+
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
941+
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
942+
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
943+
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
944+
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
945+
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
946+
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
947+
0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41, 0x41,
948+
/* end of SKID payload */
949+
0x30, 0x0A, 0x06, 0x08, 0x2A, 0x86, 0x48, 0xCE,
950+
0x3D, 0x04, 0x03, 0x02, 0x03, 0x09, 0x00, 0x30,
951+
0x06, 0x02, 0x01, 0x01, 0x02, 0x01, 0x01
952+
};
953+
954+
WOLFSSL_X509* req = NULL;
955+
WOLFSSL_BIO* bio = NULL;
956+
957+
/* Step 1: Parse the crafted CSR — this should succeed (the parser
958+
* dynamically allocates subjKeyId to the parsed size). */
959+
req = wolfSSL_X509_REQ_d2i(NULL, crafted_csr_der,
960+
(int)sizeof(crafted_csr_der));
961+
ExpectNotNull(req);
962+
963+
/* Step 2: Attempt to re-encode. Before the fix, this triggered a
964+
* heap buffer overflow in ReqCertFromX509() writing 64 bytes into
965+
* cert->skid[32]. With the fix, it must return failure. */
966+
bio = wolfSSL_BIO_new(wolfSSL_BIO_s_mem());
967+
ExpectNotNull(bio);
968+
ExpectIntNE(wolfSSL_i2d_X509_REQ_bio(bio, req), WOLFSSL_SUCCESS);
969+
970+
wolfSSL_BIO_free(bio);
971+
wolfSSL_X509_free(req);
972+
#endif
973+
return EXPECT_RESULT();
974+
}

‎tests/api/test_x509.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,15 @@ int test_x509_set_serialNumber(void);
2828
int test_x509_verify_cert_hostname_check(void);
2929
int test_x509_time_field_overread_via_tls(void);
3030
int test_x509_CertFromX509_akid_overflow(void);
31+
int test_x509_ReqCertFromX509_skid_overflow(void);
3132

3233
#define TEST_X509_DECLS \
3334
TEST_DECL_GROUP("x509", test_x509_rfc2818_verification_callback), \
3435
TEST_DECL_GROUP("x509", test_x509_GetCAByAKID), \
3536
TEST_DECL_GROUP("x509", test_x509_set_serialNumber), \
3637
TEST_DECL_GROUP("x509", test_x509_verify_cert_hostname_check), \
3738
TEST_DECL_GROUP("x509", test_x509_time_field_overread_via_tls), \
38-
TEST_DECL_GROUP("x509", test_x509_CertFromX509_akid_overflow)
39+
TEST_DECL_GROUP("x509", test_x509_CertFromX509_akid_overflow), \
40+
TEST_DECL_GROUP("x509", test_x509_ReqCertFromX509_skid_overflow)
3941

4042
#endif /* WOLFCRYPT_TEST_X509_H */

‎wolfssl/version.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@
2828
extern "C" {
2929
#endif
3030

31-
#define LIBWOLFSSL_VERSION_STRING "5.9.1"
32-
#define LIBWOLFSSL_VERSION_HEX 0x05009001
31+
#define LIBWOLFSSL_VERSION_STRING "5.9.0"
32+
#define LIBWOLFSSL_VERSION_HEX 0x05009000
3333

3434
#ifdef __cplusplus
3535
}

0 commit comments

Comments
 (0)