Bounds-check the RecipientInfo SET length in wc_PKCS7_ParseToRecipientInfoSet()

Author: Frauschi

tests/api/test_pkcs7.c

@@ -2711,6 +2711,72 @@ int test_wc_PKCS7_DecodeEnvelopedData_stream(void)
 } /* END test_wc_PKCS7_DecodeEnvelopedData_stream() */
 
 
+/*
+ * Regression test: a PKCS#7 EnvelopedData with a forged RecipientInfo SET
+ * length (parsed via GetSet_ex with NO_USER_CHECK) must not drive an
+ * uncapped heap allocation through wc_PKCS7_GrowStream(). The decoder
+ * must reject the oversized allocation rather than attempting it.
+ */
+int test_wc_PKCS7_DecodeEnvelopedData_forgedRecipientSetLen(void)
+{
+    EXPECT_DECLS;
+#if defined(HAVE_PKCS7) && !defined(NO_RSA) && !defined(NO_PKCS7_STREAM)
+    /* Crafted ContentInfo/EnvelopedData header. All lengths use the
+     * 4-byte long form for clarity. The RecipientInfo SET length is
+     * forged to 0x01000001 (16 MB + 1), which exceeds the default
+     * WOLFSSL_PKCS7_MAX_STREAM_ALLOC cap and should be rejected before
+     * any allocation succeeds. The body after the SET header is never
+     * consumed because the decoder fails at the GrowStream() cap. */
+    static const byte forged[] = {
+        /* ContentInfo SEQUENCE, body length 0x01000021 */
+        0x30, 0x84, 0x01, 0x00, 0x00, 0x21,
+        /* OID 1.2.840.113549.1.7.3 (id-envelopedData) */
+        0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D,
+        0x01, 0x07, 0x03,
+        /* [0] EXPLICIT content, body length 0x01000016 */
+        0xA0, 0x84, 0x01, 0x00, 0x00, 0x16,
+        /* EnvelopedData SEQUENCE, body length 0x01000010 */
+        0x30, 0x84, 0x01, 0x00, 0x00, 0x10,
+        /* version INTEGER 0 */
+        0x02, 0x01, 0x00,
+        /* Forged RecipientInfo SET header: length = 0x01000001 */
+        0x31, 0x84, 0x01, 0x00, 0x00, 0x01,
+        /* Padding so that header-parsing states can buffer their
+         * required lookahead without returning WC_PKCS7_WANT_READ_E.
+         * These bytes are never interpreted. */
+        0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+        0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+        0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+        0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+        0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+        0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+        0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+        0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF
+    };
+    PKCS7* pkcs7 = NULL;
+    byte   out[32];
+    int    ret;
+
+    ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId));
+    ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, (byte*)client_cert_der_2048,
+        sizeof_client_cert_der_2048), 0);
+    ExpectIntEQ(wc_PKCS7_SetKey(pkcs7, (byte*)client_key_der_2048,
+        sizeof_client_key_der_2048), 0);
+
+    ret = wc_PKCS7_DecodeEnvelopedData(pkcs7, (byte*)forged,
+        (word32)sizeof(forged), out, (word32)sizeof(out));
+    /* Must NOT return WC_PKCS7_WANT_READ_E (which would imply the
+     * oversized allocation succeeded and the decoder is waiting for
+     * the around 16 MB of SET body). Must NOT return 0 / positive length.
+     * Expected: BUFFER_E from the GrowStream cap. */
+    ExpectIntEQ(ret, WC_NO_ERR_TRACE(BUFFER_E));
+
+    wc_PKCS7_Free(pkcs7);
+#endif
+    return EXPECT_RESULT();
+} /* END test_wc_PKCS7_DecodeEnvelopedData_forgedRecipientSetLen() */
+
+
 /*
  * Testing wc_PKCS7_DecodeEnvelopedData with streaming
  */

tests/api/test_pkcs7.h

@@ -65,6 +65,7 @@ int test_wc_PKCS7_SetOriEncryptCtx(void);
 int test_wc_PKCS7_SetOriDecryptCtx(void);
 int test_wc_PKCS7_DecodeCompressedData(void);
 int test_wc_PKCS7_DecodeEnvelopedData_multiple_recipients(void);
+int test_wc_PKCS7_DecodeEnvelopedData_forgedRecipientSetLen(void);
 int test_wc_PKCS7_VerifySignedData_PKCS7ContentSeq(void);
 int test_wc_PKCS7_VerifySignedData_IndefLenOOB(void);
 
@@ -129,7 +130,8 @@ int test_wc_PKCS7_VerifySignedData_IndefLenOOB(void);
     TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_DecodeOneSymmetricKey),       \
     TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_SetOriEncryptCtx),            \
     TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_SetOriDecryptCtx),            \
-    TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_DecodeEnvelopedData_multiple_recipients)
+    TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_DecodeEnvelopedData_multiple_recipients), \
+    TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_DecodeEnvelopedData_forgedRecipientSetLen)
 
 #define TEST_PKCS7_SIGNED_ENCRYPTED_DATA_DECLS                              \
     TEST_DECL_GROUP("pkcs7_sed", test_wc_PKCS7_signed_enveloped)

wolfcrypt/src/pkcs7.c

@@ -113,6 +113,17 @@ struct PKCS7SignerInfo {
 
 #ifndef NO_PKCS7_STREAM
 
+/* Hard upper bound on a single PKCS7 streaming buffer allocation. Guards
+ * wc_PKCS7_GrowStream against attacker-controlled ASN.1 lengths that were
+ * parsed with NO_USER_CHECK and would otherwise drive allocations up to
+ * around 2GB (e.g. via a forged RecipientInfo SET length). 16 MB is well above
+ * any legitimate RecipientInfo / encoded-attribute size but small enough
+ * that a forged length fails allocation on constrained targets and is
+ * rejected on larger ones. */
+#ifndef WOLFSSL_PKCS7_MAX_STREAM_ALLOC
+    #define WOLFSSL_PKCS7_MAX_STREAM_ALLOC (16 * 1024 * 1024)
+#endif
+
 #define MAX_PKCS7_STREAM_BUFFER 256
 struct PKCS7State {
     byte* tmpCert;
@@ -274,6 +285,16 @@ static void wc_PKCS7_FreeStream(wc_PKCS7* pkcs7)
 static int wc_PKCS7_GrowStream(wc_PKCS7* pkcs7, word32 newSz)
 {
     byte* pt;
+
+    /* Guard against attacker-controlled ASN.1 lengths reaching this
+     * allocation. Several callers parse lengths with NO_USER_CHECK and
+     * pass them here unvalidated (e.g. wc_PKCS7_ParseToRecipientInfoSet
+     * on a forged RecipientInfo SET header). */
+    if (newSz > WOLFSSL_PKCS7_MAX_STREAM_ALLOC) {
+        WOLFSSL_MSG("PKCS7 streaming allocation exceeds maximum");
+        return BUFFER_E;
+    }
+
     pt = (byte*)XMALLOC(newSz, pkcs7->heap, DYNAMIC_TYPE_PKCS7);
     if (pt == NULL) {
         return MEMORY_E;
@@ -13096,6 +13117,22 @@ static int wc_PKCS7_ParseToRecipientInfoSet(wc_PKCS7* pkcs7, byte* in,
                         NO_USER_CHECK) < 0)
                 ret = ASN_PARSE_E;
 
+            /* GetSet_ex is called with NO_USER_CHECK, which skips the
+             * (idx + length > maxIdx) bounds check in GetLength_ex. In
+             * non-streaming mode, validate the SET length against the
+             * remaining input buffer; in streaming mode the length flows
+             * into pkcs7->stream->expected and then wc_PKCS7_GrowStream,
+             * where it is capped by WOLFSSL_PKCS7_MAX_STREAM_ALLOC. */
+            if (ret == 0 && length < 0)
+                ret = ASN_PARSE_E;
+        #ifdef NO_PKCS7_STREAM
+            if (ret == 0 &&
+                    (*idx > pkiMsgSz ||
+                     (word32)length > pkiMsgSz - *idx)) {
+                ret = ASN_PARSE_E;
+            }
+        #endif
+
             if (ret < 0)
                 break;