Merge pull request #9758 from LinuxJedi/lxj-fixes

Minor fixes to EVP and PKCS12 code

Author: SparkiDev

tests/api/test_evp_cipher.c

@@ -2702,3 +2702,82 @@ int test_wolfSSL_EVP_mdc2(void)
     return EXPECT_RESULT();
 }
 
+/* Test for integer overflow in EVP AEAD AAD accumulation.
+ *
+ * wolfSSL_EVP_CipherUpdate_GCM_AAD (and the CCM/ARIA variants) compute
+ * allocation sizes as (ctx->authInSz + inl) where both operands are int.
+ * Repeated AAD calls can accumulate authInSz to a value where adding inl
+ * overflows the signed int sum. The overflowed value is then cast to size_t
+ * for XMALLOC/XREALLOC, producing either:
+ *   - A huge allocation on 64-bit (masking the bug as MEMORY_E), or
+ *   - A potential heap buffer overflow on 32-bit if the wrapped size is small
+ *     enough to succeed but the subsequent XMEMCPY uses the original large
+ *     authInSz offset.
+ *
+ * This test simulates the overflow condition by directly setting authInSz near
+ * INT_MAX after legitimate initialization, then calling EVP_EncryptUpdate with
+ * AAD that triggers the overflow. A properly-fixed implementation should detect
+ * the overflow and return WOLFSSL_FAILURE before attempting the allocation.
+ */
+int test_evp_cipher_aead_aad_overflow(void)
+{
+    EXPECT_DECLS;
+#if defined(OPENSSL_EXTRA) && !defined(NO_AES) && defined(HAVE_AESGCM) && \
+    defined(WOLFSSL_AES_256) && !defined(HAVE_SELFTEST) && !defined(HAVE_FIPS) && \
+    !defined(WOLFSSL_AESGCM_STREAM)
+
+    WOLFSSL_EVP_CIPHER_CTX *ctx = NULL;
+    byte key[32] = {0};
+    byte iv[12] = {0};
+    byte aad[32] = {0};
+    int outl = 0;
+    int savedAuthInSz;
+
+    /* Initialize AES-256-GCM encryption context */
+    ctx = EVP_CIPHER_CTX_new();
+    ExpectNotNull(ctx);
+    ExpectIntEQ(WOLFSSL_SUCCESS, EVP_EncryptInit_ex(ctx, EVP_aes_256_gcm(),
+        NULL, key, iv));
+
+    /* Feed a small legitimate AAD to allocate authIn */
+    ExpectIntEQ(WOLFSSL_SUCCESS, EVP_EncryptUpdate(ctx, NULL, &outl, aad, 16));
+
+    if (EXPECT_SUCCESS()) {
+        ExpectIntEQ(ctx->authInSz, 16);
+
+        /* Simulate accumulated AAD near INT_MAX.
+         * In a real attack scenario, an attacker controlling AAD input to a
+         * server could accumulate authInSz toward INT_MAX through many calls.
+         * We set it directly to avoid needing ~2GB of actual allocations.
+         */
+        savedAuthInSz = ctx->authInSz;
+        ctx->authInSz = INT_MAX - 16;
+
+        /* Attempt AAD update that causes overflow:
+         *   (INT_MAX - 16) + 32 = INT_MAX + 16
+         * This overflows signed int (undefined behavior in C). The result:
+         *   - As signed int: wraps to INT_MIN + 15 (on 2's complement)
+         *   - Cast to size_t on 64-bit: ~0xFFFFFFFF8000000F (huge)
+         *   - Cast to size_t on 32-bit: ~0x8000000F (~2GB)
+         *
+         * With no overflow check, the code proceeds to XREALLOC with the
+         * wrapped size. On 64-bit this fails (MEMORY_E), accidentally
+         * preventing corruption. On 32-bit, if the allocation succeeds,
+         * XMEMCPY writes at offset (INT_MAX - 16) into the buffer, causing
+         * heap corruption.
+         */
+        ExpectIntNE(WOLFSSL_SUCCESS,
+            EVP_EncryptUpdate(ctx, NULL, &outl, aad, 32));
+
+        /* Restore authInSz so cleanup doesn't operate on corrupted state */
+        if (ctx != NULL)
+            ctx->authInSz = savedAuthInSz;
+    }
+
+    EVP_CIPHER_CTX_free(ctx);
+
+#endif /* OPENSSL_EXTRA && HAVE_AESGCM && WOLFSSL_AES_256 */
+    return EXPECT_RESULT();
+}
+
+

tests/api/test_evp_cipher.h

@@ -63,6 +63,7 @@ int test_wolfSSL_EVP_rc4(void);
 int test_wolfSSL_EVP_enc_null(void);
 int test_wolfSSL_EVP_rc2_cbc(void);
 int test_wolfSSL_EVP_mdc2(void);
+int test_evp_cipher_aead_aad_overflow(void);
 
 #define TEST_EVP_CIPHER_DECLS                                               \
     TEST_DECL_GROUP("evp_cipher", test_wolfSSL_EVP_CIPHER_CTX),             \
@@ -103,6 +104,7 @@ int test_wolfSSL_EVP_mdc2(void);
     TEST_DECL_GROUP("evp_cipher", test_wolfSSL_EVP_rc4),                    \
     TEST_DECL_GROUP("evp_cipher", test_wolfSSL_EVP_enc_null),               \
     TEST_DECL_GROUP("evp_cipher", test_wolfSSL_EVP_rc2_cbc),                \
-    TEST_DECL_GROUP("evp_cipher", test_wolfSSL_EVP_mdc2)
+    TEST_DECL_GROUP("evp_cipher", test_wolfSSL_EVP_mdc2),                  \
+    TEST_DECL_GROUP("evp_cipher", test_evp_cipher_aead_aad_overflow)
 
 #endif /* WOLFCRYPT_TEST_EVP_CIPHER_H */

tests/api/test_pkcs12.c

@@ -196,3 +196,42 @@ int test_wc_PKCS12_create(void)
     return EXPECT_RESULT();
 }
 
+int test_wc_d2i_PKCS12_bad_mac_salt(void)
+{
+    EXPECT_DECLS;
+#if !defined(NO_ASN) && !defined(NO_PWDBASED) && defined(HAVE_PKCS12) \
+    && !defined(NO_FILESYSTEM) && !defined(NO_RSA) \
+    && !defined(NO_AES) && !defined(NO_SHA) && !defined(NO_SHA256)
+    WC_PKCS12* pkcs12 = NULL;
+    unsigned char der[FOURK_BUF * 2];
+    int derSz = 0;
+    const char p12_f[] = "./certs/test-servercert.p12";
+    XFILE f = XBADFILE;
+    int i;
+    int found = 0;
+
+    ExpectTrue((f = XFOPEN(p12_f, "rb")) != XBADFILE);
+    ExpectIntGT(derSz = (int)XFREAD(der, 1, sizeof(der), f), 0);
+    if (f != XBADFILE)
+        XFCLOSE(f);
+
+    /* Scan backward within the last 100 bytes to find the MAC salt
+     * OCTET STRING (tag 0x04, length 0x08 for a typical 8-byte salt).
+     * Corrupt its length so that saltSz + curIdx > totalSz, triggering
+     * the error path in GetSignData() after salt allocation. */
+    for (i = derSz - 2; i >= 0 && i >= derSz - 100; i--) {
+        if (der[i] == 0x04 && der[i + 1] == 0x08) {
+            der[i + 1] = 0xFF;
+            found = 1;
+            break;
+        }
+    }
+    ExpectIntEQ(found, 1);
+
+    ExpectNotNull(pkcs12 = wc_PKCS12_new());
+    ExpectIntNE(wc_d2i_PKCS12(der, (word32)derSz, pkcs12), 0);
+    wc_PKCS12_free(pkcs12);
+#endif
+    return EXPECT_RESULT();
+}
+

tests/api/test_pkcs12.h

@@ -26,9 +26,11 @@
 
 int test_wc_i2d_PKCS12(void);
 int test_wc_PKCS12_create(void);
+int test_wc_d2i_PKCS12_bad_mac_salt(void);
 
 #define TEST_PKCS12_DECLS                                       \
     TEST_DECL_GROUP("pkcs12", test_wc_i2d_PKCS12),              \
-    TEST_DECL_GROUP("pkcs12", test_wc_PKCS12_create)
+    TEST_DECL_GROUP("pkcs12", test_wc_PKCS12_create),            \
+    TEST_DECL_GROUP("pkcs12", test_wc_d2i_PKCS12_bad_mac_salt)
 
 #endif /* WOLFCRYPT_TEST_PKCS12_H */

wolfcrypt/src/evp.c

@@ -745,6 +745,10 @@ static int wolfSSL_EVP_CipherUpdate_GCM_AAD(WOLFSSL_EVP_CIPHER_CTX *ctx,
         const unsigned char *in, int inl) {
     if (in && inl > 0) {
         byte* tmp;
+        if (inl > INT_MAX - ctx->authInSz) {
+            WOLFSSL_MSG("AuthIn overflow");
+            return BAD_FUNC_ARG;
+        }
     #ifdef WOLFSSL_NO_REALLOC
         tmp = (byte*)XMALLOC((size_t)(ctx->authInSz + inl), NULL,
                 DYNAMIC_TYPE_OPENSSL);
@@ -789,6 +793,9 @@ static int wolfSSL_EVP_CipherUpdate_GCM(WOLFSSL_EVP_CIPHER_CTX *ctx,
             /* Buffer input for one-shot API */
             if (inl > 0) {
                 byte* tmp;
+                if ((int)inl > INT_MAX - ctx->authBufferLen) {
+                    return MEMORY_E;
+                }
             #ifdef WOLFSSL_NO_REALLOC
                 tmp = (byte*)XMALLOC((size_t)(ctx->authBufferLen + inl), NULL,
                         DYNAMIC_TYPE_OPENSSL);
@@ -872,6 +879,10 @@ static int wolfSSL_EVP_CipherUpdate_CCM_AAD(WOLFSSL_EVP_CIPHER_CTX *ctx,
         const unsigned char *in, int inl) {
     if (in && inl > 0) {
         byte* tmp;
+        if (inl > INT_MAX - ctx->authInSz) {
+            WOLFSSL_MSG("AuthIn overflow");
+            return BAD_FUNC_ARG;
+        }
     #ifdef WOLFSSL_NO_REALLOC
         tmp = (byte*)XMALLOC((size_t)(ctx->authInSz + inl), NULL,
                 DYNAMIC_TYPE_OPENSSL);
@@ -908,6 +919,9 @@ static int wolfSSL_EVP_CipherUpdate_CCM(WOLFSSL_EVP_CIPHER_CTX *ctx,
         /* Buffer input for one-shot API */
         if (inl > 0) {
             byte* tmp;
+            if (inl > INT_MAX - ctx->authBufferLen) {
+                return MEMORY_E;
+            }
         #ifdef WOLFSSL_NO_REALLOC
             tmp = (byte*)XMALLOC((size_t)(ctx->authBufferLen + inl), NULL,
                     DYNAMIC_TYPE_OPENSSL);
@@ -951,8 +965,12 @@ static int wolfSSL_EVP_CipherUpdate_AriaGCM_AAD(WOLFSSL_EVP_CIPHER_CTX *ctx,
 {
     if (in && inl > 0) {
         byte* tmp;
+        if (inl > INT_MAX - ctx->authInSz) {
+            WOLFSSL_MSG("AuthIn overflow");
+            return BAD_FUNC_ARG;
+        }
     #ifdef WOLFSSL_NO_REALLOC
-        tmp = (byte*)XMALLOC((size_t)ctx->authInSz + inl, NULL,
+        tmp = (byte*)XMALLOC((size_t)(ctx->authInSz + inl), NULL,
                 DYNAMIC_TYPE_OPENSSL);
         if (tmp != NULL) {
             XMEMCPY(tmp, ctx->authIn, (size_t)ctx->authInSz);
@@ -961,7 +979,7 @@ static int wolfSSL_EVP_CipherUpdate_AriaGCM_AAD(WOLFSSL_EVP_CIPHER_CTX *ctx,
         }
     #else
         tmp = (byte*)XREALLOC(ctx->authIn,
-                (size_t)ctx->authInSz + inl, NULL, DYNAMIC_TYPE_OPENSSL);
+                (size_t)(ctx->authInSz + inl), NULL, DYNAMIC_TYPE_OPENSSL);
     #endif
         if (tmp) {
             ctx->authIn = tmp;

wolfcrypt/src/pkcs12.c

@@ -509,6 +509,7 @@ static int GetSignData(WC_PKCS12* pkcs12, const byte* mem, word32* idx,
     if (ret != 0) {
         if (mac) {
             XFREE(mac->digest, pkcs12->heap, DYNAMIC_TYPE_DIGEST);
+            XFREE(mac->salt, pkcs12->heap, DYNAMIC_TYPE_SALT);
             XFREE(mac, pkcs12->heap, DYNAMIC_TYPE_PKCS);
         }
     }
@@ -1134,6 +1135,7 @@ static byte* PKCS12_ConcatenateContent(WC_PKCS12* pkcs12,byte* mergedData,
 {
     byte* oldContent;
     word32 oldContentSz;
+    word32 newSz;
 
     (void)pkcs12;
 
@@ -1145,14 +1147,19 @@ static byte* PKCS12_ConcatenateContent(WC_PKCS12* pkcs12,byte* mergedData,
     oldContentSz = *mergedSz;
 
     /* re-allocate new buffer to fit appended data */
-    mergedData = (byte*)XMALLOC(oldContentSz + inSz, pkcs12->heap,
+    if (WC_SAFE_SUM_WORD32(oldContentSz, inSz, newSz) == 0) {
+        XFREE(oldContent, pkcs12->heap, DYNAMIC_TYPE_PKCS);
+        return NULL;
+    }
+
+    mergedData = (byte*)XMALLOC(newSz, pkcs12->heap,
             DYNAMIC_TYPE_PKCS);
     if (mergedData != NULL) {
         if (oldContent != NULL) {
             XMEMCPY(mergedData, oldContent, oldContentSz);
         }
         XMEMCPY(mergedData + oldContentSz, in, inSz);
-        *mergedSz += inSz;
+        *mergedSz = newSz;
     }
     XFREE(oldContent, pkcs12->heap, DYNAMIC_TYPE_PKCS);