Fix wolfSSL_sk_X509_OBJECT_deep_copy to check bounds

Author: embhorn

src/x509.c

@@ -11712,8 +11712,18 @@ WOLF_STACK_OF(WOLFSSL_X509_OBJECT)* wolfSSL_sk_X509_OBJECT_deep_copy(
             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) {
+                    if (req->subjKeyId) {
+                        XMEMCPY(cert->skid, req->subjKeyId,
+                                req->subjKeyIdSz);
+                    }
+                    cert->skidSz = (int)req->subjKeyIdSz;
+                }
+                else {
+                    WOLFSSL_MSG("Subject Key ID too large");
+                    WOLFSSL_ERROR_VERBOSE(BUFFER_E);
+                    ret = WOLFSSL_FAILURE;
+                }
             }
             if (req->keyUsageSet)
                 cert->keyUsage = req->keyUsage;

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();
+}

tests/api/test_x509.h

@@ -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 */

wolfssl/version.h

@@ -28,8 +28,8 @@
 extern "C" {
 #endif
 
-#define LIBWOLFSSL_VERSION_STRING "5.9.1"
-#define LIBWOLFSSL_VERSION_HEX 0x05009001
+#define LIBWOLFSSL_VERSION_STRING "5.9.0"
+#define LIBWOLFSSL_VERSION_HEX 0x05009000
 
 #ifdef __cplusplus
 }