From 84fb0f694cfaac4187c0beba4298a4a8a7995235 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 13 Apr 2026 14:30:06 +0200 Subject: [PATCH 01/11] Fix various range and size bugs in PKCS#7 code --- tests/api.c | 102 ++++++++++++++++++++++++++++++++++++++++++ wolfcrypt/src/pkcs7.c | 64 +++++++++++++++++++++++--- 2 files changed, 159 insertions(+), 7 deletions(-) diff --git a/tests/api.c b/tests/api.c index db7b595522..7e20480484 100644 --- a/tests/api.c +++ b/tests/api.c @@ -35859,6 +35859,107 @@ static int test_pkcs7_ori_oversized_oid(void) return EXPECT_RESULT(); } +/* ORI callback that flags if oriValueSz looks like an underflow (>= 0x80000000) */ +#if defined(HAVE_PKCS7) && !defined(WOLFSSL_NO_MALLOC) +static int test_ori_underflow_cb(wc_PKCS7* pkcs7, byte* oriType, + word32 oriTypeSz, byte* oriValue, + word32 oriValueSz, byte* decryptedKey, + word32* decryptedKeySz, void* ctx) +{ + int* called = (int*)ctx; + (void)pkcs7; (void)oriType; (void)oriTypeSz; + (void)oriValue; (void)decryptedKey; (void)decryptedKeySz; + if (called != NULL) + *called = (int)oriValueSz; /* record what we received */ + return -1; +} +#endif + +/* Test: PKCS#7 ORI must reject when OID consumption exceeds the [4] implicit + * SEQUENCE length (integer underflow in oriValueSz computation). + * + * With implicit tagging, [4] CONSTRUCTED replaces the SEQUENCE tag, so + * wc_PKCS7_DecryptOri reads seqSz directly from the [4] length field. + * We set [4] length = 5 while the OID inside consumes 22 bytes + * (tag + length + 20 content), triggering oriValueSz = 5 - 22 = underflow. + * + * The buffer includes a dummy EncryptedContentInfo after the RecipientInfos + * so the total message is large enough for the PKCS7 streaming code (which + * requests the full remaining message before parsing the ORI). */ +static int test_pkcs7_ori_seqsz_underflow(void) +{ + EXPECT_DECLS; +#if defined(HAVE_PKCS7) && !defined(WOLFSSL_NO_MALLOC) + wc_PKCS7* p7 = NULL; + byte out[256]; + int cbCalled = 0; + + /* + * Byte layout (all outer lengths match actual byte counts on wire): + * + * OID inside [4]: 06 14 <20 bytes> = 22 bytes + * [4] (declared len 5, actual content 22): + * a4 05 <22 bytes> = 24 bytes on wire + * SET: 31 18 <24 bytes> = 26 bytes on wire + * version: 02 01 00 = 3 bytes + * EncryptedContentInfo (filler, never parsed): + * 30 0b { 06 09 <9 bytes OID> } = 13 bytes on wire + * EnvelopedData: 30 2a <3+26+13=42 bytes> = 44 bytes on wire + * [0] EXPLICIT: a0 2c <44 bytes> = 46 bytes on wire + * OID(envelopedData): 06 09 <9 bytes> = 11 bytes on wire + * ContentInfo: 30 39 <11+46=57 bytes> = 59 bytes total + */ + static const byte poc[] = { + /* ContentInfo SEQUENCE (length 57) */ + 0x30, 0x39, + /* contentType = envelopedData 1.2.840.113549.1.7.3 */ + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x03, + /* [0] EXPLICIT (length 44) */ + 0xa0, 0x2c, + /* EnvelopedData SEQUENCE (length 42) */ + 0x30, 0x2a, + /* version = 0 */ + 0x02, 0x01, 0x00, + /* RecipientInfos SET (length 24) */ + 0x31, 0x18, + /* [4] CONSTRUCTED = ORI implicit SEQUENCE, declared len 5 */ + /* Actual OID is 22 bytes -> exceeds declared 5 */ + 0xa4, 0x05, + /* OID: tag=06, len=0x14(20), content=20 bytes = 22 total */ + 0x06, 0x14, + 0x2a, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, + 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, + 0x12, 0x13, 0x14, 0x15, + /* EncryptedContentInfo SEQUENCE (length 11) - filler so + * streaming has enough data; never actually parsed because + * DecryptOri fails before we get here */ + 0x30, 0x0b, + /* contentType = data 1.2.840.113549.1.7.1 */ + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, + 0x01 + }; + + p7 = wc_PKCS7_New(NULL, INVALID_DEVID); + ExpectNotNull(p7); + if (p7 != NULL) { + wc_PKCS7_SetOriDecryptCb(p7, test_ori_underflow_cb); + wc_PKCS7_SetOriDecryptCtx(p7, &cbCalled); + + /* Must return an error before the callback sees an underflowed size */ + ExpectIntLT(wc_PKCS7_DecodeEnvelopedData(p7, (byte*)poc, sizeof(poc), + out, sizeof(out)), 0); + + /* The callback must NOT have been invoked with a wrapped oriValueSz. + * cbCalled == 0 means the callback was never reached (ideal). + * cbCalled < 0 would indicate the underflow was passed through. */ + ExpectIntGE(cbCalled, 0); + + wc_PKCS7_Free(p7); + } +#endif + return EXPECT_RESULT(); +} + /* Dilithium verify_ctx_msg must reject absurdly large msgLen */ static int test_dilithium_hash(void) { @@ -36800,6 +36901,7 @@ TEST_CASE testCases[] = { TEST_DECL(test_ed448_rejects_identity_key), TEST_DECL(test_pkcs7_decode_encrypted_outputsz), TEST_DECL(test_pkcs7_ori_oversized_oid), + TEST_DECL(test_pkcs7_ori_seqsz_underflow), TEST_DECL(test_pkcs7_padding), #if defined(WOLFSSL_SNIFFER) && defined(WOLFSSL_SNIFFER_CHAIN_INPUT) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 36b817ed51..7678bed965 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -10769,15 +10769,19 @@ static int wc_PKCS7_DecryptKtri(wc_PKCS7* pkcs7, byte* in, word32 inSz, if (GetLength(pkiMsg, idx, &length, pkiMsgSz) < 0) return ASN_PARSE_E; - if ((word32)keyIdSize > pkiMsgSz - (*idx)) + /* Validate SKID container and keyIdSize against buffer */ + if ((word32)length > pkiMsgSz - (*idx)) return BUFFER_E; + if (length < keyIdSize) + return ASN_PARSE_E; + /* if we found correct recipient, SKID will match */ if (XMEMCMP(pkiMsg + (*idx), pkcs7->issuerSubjKeyId, (word32)keyIdSize) == 0) { *recipFound = 1; } - (*idx) += (word32)keyIdSize; + (*idx) += (word32)length; } if (GetAlgoId(pkiMsg, idx, &encOID, oidKeyType, pkiMsgSz) < 0) @@ -11054,6 +11058,14 @@ static int wc_PKCS7_KariGetOriginatorIdentifierOrKey(WC_PKCS7_KARI* kari, if (GetLength(pkiMsg, idx, &length, pkiMsgSz) < 0) return ASN_PARSE_E; + /* BIT STRING must have at least unused-bits byte + 1 byte of content */ + if (length < 2) + return ASN_PARSE_E; + + /* Validate BIT STRING content is within input buffer */ + if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx) + return ASN_PARSE_E; + if (GetASNTag(pkiMsg, idx, &tag, pkiMsgSz) < 0) return ASN_EXPECT_0_E; @@ -11533,9 +11545,22 @@ static int wc_PKCS7_DecryptOri(wc_PKCS7* pkcs7, byte* in, word32 inSz, XMEMCPY(oriOID, pkiMsg + *idx, (word32)oriOIDSz); *idx += (word32)oriOIDSz; + /* Validate OID did not consume more than the SEQUENCE declared */ + if ((*idx - tmpIdx) > (word32)seqSz) { + WOLFSSL_MSG("ORI oriType OID exceeds SEQUENCE boundary"); + return ASN_PARSE_E; + } + /* get oriValue, increment idx */ oriValue = pkiMsg + *idx; oriValueSz = (word32)seqSz - (*idx - tmpIdx); + + /* Validate oriValue region is within input buffer */ + if (*idx > pkiMsgSz || oriValueSz > pkiMsgSz - *idx) { + WOLFSSL_MSG("ORI oriValue exceeds input buffer"); + return ASN_PARSE_E; + } + *idx += oriValueSz; /* pass oriOID and oriValue to user callback, expect back @@ -11713,6 +11738,12 @@ static int wc_PKCS7_DecryptPwri(wc_PKCS7* pkcs7, byte* in, word32 inSz, return ASN_PARSE_E; } + /* Validate IV is within input buffer */ + if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx) { + XFREE(salt, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + return ASN_PARSE_E; + } + XMEMCPY(tmpIv, pkiMsg + (*idx), (word32)length); *idx += (word32)length; @@ -11732,6 +11763,12 @@ static int wc_PKCS7_DecryptPwri(wc_PKCS7* pkcs7, byte* in, word32 inSz, return ASN_PARSE_E; } + /* Validate EncryptedKey is within input buffer */ + if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx) { + XFREE(salt, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + return ASN_PARSE_E; + } + /* allocate temporary space for decrypted key */ cekSz = (word32)length; cek = (byte*)XMALLOC(cekSz, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); @@ -11818,7 +11855,7 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz, byte* keyId = NULL; const byte* datePtr = NULL; byte dateFormat, tag; - word32 keyIdSz, kekIdSz, keyWrapOID, localIdx; + word32 keyIdSz, kekIdSz, kekIdEnd, keyWrapOID, localIdx; int ret = 0; byte* pkiMsg = in; @@ -11844,6 +11881,11 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz, return ASN_PARSE_E; kekIdSz = (word32)length; + kekIdEnd = *idx + kekIdSz; + + /* Validate KEKIdentifier boundary is within input buffer */ + if (kekIdEnd < *idx || kekIdEnd > pkiMsgSz) + return ASN_PARSE_E; if (GetASNTag(pkiMsg, idx, &tag, pkiMsgSz) < 0) return ASN_PARSE_E; @@ -11854,6 +11896,10 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz, if (GetLength(pkiMsg, idx, &length, pkiMsgSz) < 0) return ASN_PARSE_E; + /* Validate keyIdentifier is within input buffer */ + if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx) + return ASN_PARSE_E; + /* save keyIdentifier and length */ keyId = pkiMsg + *idx; keyIdSz = (word32)length; @@ -11861,10 +11907,10 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz, /* may have OPTIONAL GeneralizedTime */ localIdx = *idx; - if ((*idx < kekIdSz) && GetASNTag(pkiMsg, &localIdx, &tag, + if ((*idx < kekIdEnd) && GetASNTag(pkiMsg, &localIdx, &tag, pkiMsgSz) == 0 && tag == ASN_GENERALIZED_TIME) { - if (wc_GetDateInfo(pkiMsg + *idx, (int)pkiMsgSz, &datePtr, - &dateFormat, &dateLen) != 0) { + if (wc_GetDateInfo(pkiMsg + *idx, (int)(pkiMsgSz - *idx), + &datePtr, &dateFormat, &dateLen) != 0) { return ASN_PARSE_E; } *idx += (word32)(dateLen + 1); @@ -11876,7 +11922,7 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz, /* may have OPTIONAL OtherKeyAttribute */ localIdx = *idx; - if ((*idx < kekIdSz) && GetASNTag(pkiMsg, &localIdx, &tag, + if ((*idx < kekIdEnd) && GetASNTag(pkiMsg, &localIdx, &tag, pkiMsgSz) == 0 && tag == (ASN_SEQUENCE | ASN_CONSTRUCTED)) { if (GetSequence(pkiMsg, idx, &length, pkiMsgSz) < 0) @@ -11905,6 +11951,10 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz, if (GetLength(pkiMsg, idx, &length, pkiMsgSz) < 0) return ASN_PARSE_E; + /* Validate EncryptedKey is within input buffer */ + if (*idx > pkiMsgSz || (word32)length > pkiMsgSz - *idx) + return ASN_PARSE_E; + #ifndef NO_AES direction = AES_DECRYPTION; #else From e2167e4bbd021eb69305315098a18e3714095882 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 13 Apr 2026 15:52:56 +0200 Subject: [PATCH 02/11] add length check in PKCS#7 --- wolfcrypt/src/pkcs7.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 7678bed965..622f23d228 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -14455,7 +14455,16 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in, if (GetLength_ex(pkiMsg, &idx, &length, pkiMsgSz, 0) <= 0) { ret = ASN_PARSE_E; } - #ifndef NO_PKCS7_STREAM + + #ifdef NO_PKCS7_STREAM + /* In non-streaming mode, validate authenticatedAttributes + * length is within the input buffer. The streaming path + * handles this via wc_PKCS7_AddDataToStream instead. */ + if (ret == 0 && + (idx > pkiMsgSz || (word32)length > pkiMsgSz - idx)) { + ret = ASN_PARSE_E; + } + #else pkcs7->stream->expected = (word32)length; #endif encodedAttribSz = (word32)length + (idx - encodedAttribIdx); From 5634cfd67c3b04087a3839f564fbd111f0f8121b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 13 Apr 2026 15:53:35 +0200 Subject: [PATCH 03/11] Fix PKCS#7 regression with --enable-all and NO_PKCS7_STREAM --- tests/api/test_pkcs7.c | 34 ++++++++++++++++------------------ wolfcrypt/src/pkcs7.c | 11 ++++++----- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/tests/api/test_pkcs7.c b/tests/api/test_pkcs7.c index 3c8b59fb8e..cd9f7aa575 100644 --- a/tests/api/test_pkcs7.c +++ b/tests/api/test_pkcs7.c @@ -2397,7 +2397,8 @@ static int myCEKwrapFunc(PKCS7* pkcs7, byte* cek, word32 cekSz, byte* keyId, HAVE_AES_KEYWRAP */ -#if defined(HAVE_PKCS7) && defined(ASN_BER_TO_DER) && !defined(NO_RSA) +#if defined(HAVE_PKCS7) && defined(ASN_BER_TO_DER) && !defined(NO_RSA) && \ + !defined(NO_PKCS7_STREAM) #define MAX_TEST_DECODE_SIZE 6000 static int test_wc_PKCS7_DecodeEnvelopedData_stream_decrypt_cb(wc_PKCS7* pkcs7, const byte* output, word32 outputSz, void* ctx) { @@ -2430,7 +2431,8 @@ static int test_wc_PKCS7_DecodeEnvelopedData_stream_decrypt_cb(wc_PKCS7* pkcs7, int test_wc_PKCS7_DecodeEnvelopedData_stream(void) { EXPECT_DECLS; -#if defined(HAVE_PKCS7) && defined(ASN_BER_TO_DER) && !defined(NO_RSA) +#if defined(HAVE_PKCS7) && defined(ASN_BER_TO_DER) && !defined(NO_RSA) && \ + !defined(NO_PKCS7_STREAM) PKCS7* pkcs7 = NULL; int ret = 0; XFILE f = XBADFILE; @@ -2579,7 +2581,7 @@ int test_wc_PKCS7_EncodeDecodeEnvelopedData(void) EXPECT_DECLS; #if defined(HAVE_PKCS7) PKCS7* pkcs7 = NULL; -#ifdef ASN_BER_TO_DER +#if defined(ASN_BER_TO_DER) && !defined(NO_PKCS7_STREAM) int encodedSz = 0; #endif #ifdef ECC_TIMING_RESISTANT @@ -2784,7 +2786,7 @@ int test_wc_PKCS7_EncodeDecodeEnvelopedData(void) testSz = (int)sizeof(testVectors)/(int)sizeof(pkcs7EnvelopedVector); for (i = 0; i < testSz; i++) { - #ifdef ASN_BER_TO_DER + #if defined(ASN_BER_TO_DER) && !defined(NO_PKCS7_STREAM) encodeSignedDataStream strm; /* test setting stream mode, the first one using IO callbacks */ @@ -2950,17 +2952,11 @@ int test_wc_PKCS7_EncodeDecodeEnvelopedData(void) pkcs7->singleCert = NULL; } #ifndef NO_RSA - #if defined(NO_PKCS7_STREAM) - /* when none streaming mode is used and PKCS7 is in bad state buffer error - * is returned from kari parse which gets set to bad func arg */ - ExpectIntEQ(wc_PKCS7_DecodeEnvelopedData(pkcs7, output, - (word32)sizeof(output), decoded, (word32)sizeof(decoded)), - WC_NO_ERR_TRACE(BAD_FUNC_ARG)); - #else + /* With corrupted singleCert, decode should fail with a parse error. + * State is properly reset on error so re-decode starts from scratch. */ ExpectIntEQ(wc_PKCS7_DecodeEnvelopedData(pkcs7, output, (word32)sizeof(output), decoded, (word32)sizeof(decoded)), WC_NO_ERR_TRACE(ASN_PARSE_E)); - #endif #endif /* !NO_RSA */ if (pkcs7 != NULL) { pkcs7->singleCert = tmpBytePtr; @@ -3991,7 +3987,8 @@ int test_wc_PKCS7_Degenerate(void) } /* END test_wc_PKCS7_Degenerate() */ #if defined(HAVE_PKCS7) && !defined(NO_FILESYSTEM) && \ - defined(ASN_BER_TO_DER) && !defined(NO_DES3) && !defined(NO_SHA) + defined(ASN_BER_TO_DER) && !defined(NO_DES3) && !defined(NO_SHA) && \ + !defined(NO_PKCS7_STREAM) static byte berContent[] = { 0x30, 0x80, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x03, 0xA0, 0x80, 0x30, @@ -4182,7 +4179,7 @@ static byte berContent[] = { 0x00, 0x00, 0x00, 0x00, 0x00 }; #endif /* HAVE_PKCS7 && !NO_FILESYSTEM && ASN_BER_TO_DER && - * !NO_DES3 && !NO_SHA + * !NO_DES3 && !NO_SHA && !NO_PKCS7_STREAM */ /* @@ -4197,7 +4194,7 @@ int test_wc_PKCS7_BER(void) char fName[] = "./certs/test-ber-exp02-05-2022.p7b"; XFILE f = XBADFILE; byte der[4096]; -#ifndef NO_DES3 +#if !defined(NO_DES3) && !defined(NO_PKCS7_STREAM) byte decoded[2048]; #endif word32 derSz = 0; @@ -4242,8 +4239,9 @@ int test_wc_PKCS7_BER(void) wc_PKCS7_Free(pkcs7); pkcs7 = NULL; -#ifndef NO_DES3 - /* decode BER content */ +#if !defined(NO_DES3) && !defined(NO_PKCS7_STREAM) + /* decode BER content - requires PKCS7 streaming to handle indefinite + * length encoding in the EnvelopedData structure */ ExpectTrue((f = XFOPEN("./certs/1024/client-cert.der", "rb")) != XBADFILE); ExpectTrue((derSz = (word32)XFREAD(der, 1, sizeof(der), f)) > 0); if (f != XBADFILE) { @@ -4280,7 +4278,7 @@ int test_wc_PKCS7_BER(void) sizeof(berContent), decoded, sizeof(decoded)), WC_NO_ERR_TRACE(NOT_COMPILED_IN)); #endif wc_PKCS7_Free(pkcs7); -#endif /* !NO_DES3 */ +#endif /* !NO_DES3 && !NO_PKCS7_STREAM */ #endif return EXPECT_RESULT(); } /* END test_wc_PKCS7_BER() */ diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 622f23d228..e5a1987c56 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -10769,15 +10769,13 @@ static int wc_PKCS7_DecryptKtri(wc_PKCS7* pkcs7, byte* in, word32 inSz, if (GetLength(pkiMsg, idx, &length, pkiMsgSz) < 0) return ASN_PARSE_E; - /* Validate SKID container and keyIdSize against buffer */ + /* Validate SKID container is within buffer */ if ((word32)length > pkiMsgSz - (*idx)) return BUFFER_E; - if (length < keyIdSize) - return ASN_PARSE_E; - /* if we found correct recipient, SKID will match */ - if (XMEMCMP(pkiMsg + (*idx), pkcs7->issuerSubjKeyId, + if (length == keyIdSize && + XMEMCMP(pkiMsg + (*idx), pkcs7->issuerSubjKeyId, (word32)keyIdSize) == 0) { *recipFound = 1; } @@ -13400,6 +13398,9 @@ int wc_PKCS7_DecodeEnvelopedData(wc_PKCS7* pkcs7, byte* in, } } #else + if (ret < 0) { + wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_START); + } if (decryptedKey != NULL && ret < 0) { ForceZero(decryptedKey, MAX_ENCRYPTED_KEY_SZ); XFREE(decryptedKey, pkcs7->heap, DYNAMIC_TYPE_PKCS7); From 16e1d33f24959f3addaf9f58df737ed6edbfc81a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 13 Apr 2026 15:58:47 +0200 Subject: [PATCH 04/11] Fix invalid preprocessor guard in PKCS7 with SHA224 Also add missing ForceZero for ECDH shared secret on the heap. --- wolfcrypt/src/pkcs7.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index e5a1987c56..dcb33c7914 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -7782,7 +7782,7 @@ static int wc_PKCS7_KariGenerateKEK(WC_PKCS7_KARI* kari, WC_RNG* rng, kdfType = WC_HASH_TYPE_SHA; break; #endif - #ifndef WOLFSSL_SHA224 + #ifdef WOLFSSL_SHA224 case dhSinglePass_stdDH_sha224kdf_scheme: kdfType = WC_HASH_TYPE_SHA224; break; @@ -7804,6 +7804,7 @@ static int wc_PKCS7_KariGenerateKEK(WC_PKCS7_KARI* kari, WC_RNG* rng, #endif default: WOLFSSL_MSG("Unsupported key agreement algorithm"); + ForceZero(secret, secretSz); XFREE(secret, kari->heap, DYNAMIC_TYPE_PKCS7); return BAD_FUNC_ARG; }; @@ -7816,6 +7817,7 @@ static int wc_PKCS7_KariGenerateKEK(WC_PKCS7_KARI* kari, WC_RNG* rng, ret = NOT_COMPILED_IN; #endif + ForceZero(secret, secretSz); XFREE(secret, kari->heap, DYNAMIC_TYPE_PKCS7); return ret; } From 46f3ebb0c6fb4188fb348e1eedae0139f7119b87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Mon, 13 Apr 2026 16:20:55 +0200 Subject: [PATCH 05/11] Add missing ForceZero calls in PKCS#7 --- wolfcrypt/src/pkcs7.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index dcb33c7914..c27ce5e2b1 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -211,6 +211,9 @@ static void wc_PKCS7_ResetStream(wc_PKCS7* pkcs7) XFREE(pkcs7->stream->tag, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(pkcs7->stream->nonce, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(pkcs7->stream->buffer, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + /* stream->key is always allocated with MAX_ENCRYPTED_KEY_SZ */ + if (pkcs7->stream->key != NULL) + ForceZero(pkcs7->stream->key, MAX_ENCRYPTED_KEY_SZ); XFREE(pkcs7->stream->key, pkcs7->heap, DYNAMIC_TYPE_PKCS7); pkcs7->stream->aad = NULL; pkcs7->stream->tag = NULL; @@ -7770,6 +7773,7 @@ static int wc_PKCS7_KariGenerateKEK(WC_PKCS7_KARI* kari, WC_RNG* rng, } if (ret != 0) { + ForceZero(secret, secretSz); XFREE(secret, kari->heap, DYNAMIC_TYPE_PKCS7); return ret; } @@ -9763,6 +9767,7 @@ int wc_PKCS7_AddRecipient_PWRI(wc_PKCS7* pkcs7, byte* passwd, word32 pLen, (word32)kekKeySz); if (ret < 0) { XFREE(recip, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + ForceZero(kek, (word32)kekKeySz); XFREE(kek, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(encryptedKey, pkcs7->heap, DYNAMIC_TYPE_PKCS7); return ret; @@ -9774,6 +9779,7 @@ int wc_PKCS7_AddRecipient_PWRI(wc_PKCS7* pkcs7, byte* passwd, word32 pLen, tmpIv, (word32)kekBlockSz, encryptOID); if (ret < 0) { XFREE(recip, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + ForceZero(kek, (word32)kekKeySz); XFREE(kek, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(encryptedKey, pkcs7->heap, DYNAMIC_TYPE_PKCS7); return ret; @@ -9798,6 +9804,7 @@ int wc_PKCS7_AddRecipient_PWRI(wc_PKCS7* pkcs7, byte* passwd, word32 pLen, ret = wc_SetContentType(PWRI_KEK_WRAP, keyEncAlgoId, sizeof(keyEncAlgoId)); if (ret <= 0) { XFREE(recip, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + ForceZero(kek, (word32)kekKeySz); XFREE(kek, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(encryptedKey, pkcs7->heap, DYNAMIC_TYPE_PKCS7); return ret; @@ -9829,6 +9836,7 @@ int wc_PKCS7_AddRecipient_PWRI(wc_PKCS7* pkcs7, byte* passwd, word32 pLen, ret = wc_SetContentType(kdfOID, kdfAlgoId, sizeof(kdfAlgoId)); if (ret <= 0) { XFREE(recip, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + ForceZero(kek, (word32)kekKeySz); XFREE(kek, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(encryptedKey, pkcs7->heap, DYNAMIC_TYPE_PKCS7); return ret; @@ -9854,6 +9862,7 @@ int wc_PKCS7_AddRecipient_PWRI(wc_PKCS7* pkcs7, byte* passwd, word32 pLen, if (totalSz > MAX_RECIP_SZ) { WOLFSSL_MSG("CMS Recipient output buffer too small"); XFREE(recip, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + ForceZero(kek, (word32)kekKeySz); XFREE(kek, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(encryptedKey, pkcs7->heap, DYNAMIC_TYPE_PKCS7); return BUFFER_E; @@ -9891,7 +9900,7 @@ int wc_PKCS7_AddRecipient_PWRI(wc_PKCS7* pkcs7, byte* passwd, word32 pLen, XMEMCPY(recip->recip + idx, encryptedKey, encryptedKeySz); idx += encryptedKeySz; - ForceZero(kek, (word32)kekBlockSz); + ForceZero(kek, (word32)kekKeySz); ForceZero(encryptedKey, encryptedKeySz); XFREE(kek, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(encryptedKey, pkcs7->heap, DYNAMIC_TYPE_PKCS7); @@ -10612,7 +10621,9 @@ static int wc_PKCS7_DecryptKtri(wc_PKCS7* pkcs7, byte* in, word32 inSz, word32 pkiMsgSz = inSz; byte tag; - +#ifndef WC_NO_RSA_OAEP + word32 outKeySz = 0; +#endif #ifndef NO_PKCS7_STREAM word32 tmpIdx = *idx; #endif @@ -10921,8 +10932,8 @@ static int wc_PKCS7_DecryptKtri(wc_PKCS7* pkcs7, byte* in, word32 inSz, #ifndef WC_NO_RSA_OAEP } else { - word32 outLen = (word32)wc_RsaEncryptSize(privKey); - outKey = (byte*)XMALLOC(outLen, pkcs7->heap, + outKeySz = (word32)wc_RsaEncryptSize(privKey); + outKey = (byte*)XMALLOC(outKeySz, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); if (!outKey) { WOLFSSL_MSG("Failed to allocate out key buffer"); @@ -10936,9 +10947,9 @@ static int wc_PKCS7_DecryptKtri(wc_PKCS7* pkcs7, byte* in, word32 inSz, } keySz = wc_RsaPrivateDecrypt_ex(encryptedKey, - (word32)encryptedKeySz, outKey, outLen, privKey, - WC_RSA_OAEP_PAD, - WC_HASH_TYPE_SHA, WC_MGF1SHA1, NULL, 0); + (word32)encryptedKeySz, outKey, outKeySz, + privKey, WC_RSA_OAEP_PAD, WC_HASH_TYPE_SHA, + WC_MGF1SHA1, NULL, 0); } #endif } @@ -10961,6 +10972,7 @@ static int wc_PKCS7_DecryptKtri(wc_PKCS7* pkcs7, byte* in, word32 inSz, #ifndef WC_NO_RSA_OAEP if (encOID == RSAESOAEPk) { if (outKey) { + ForceZero(outKey, outKeySz); XFREE(outKey, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); } } @@ -10977,6 +10989,7 @@ static int wc_PKCS7_DecryptKtri(wc_PKCS7* pkcs7, byte* in, word32 inSz, #ifndef WC_NO_RSA_OAEP if (encOID == RSAESOAEPk) { if (outKey) { + ForceZero(outKey, outKeySz); XFREE(outKey, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); } } @@ -11791,6 +11804,7 @@ static int wc_PKCS7_DecryptPwri(wc_PKCS7* pkcs7, byte* in, word32 inSz, iterations, kek, (word32)kekKeySz); if (ret < 0) { XFREE(salt, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + ForceZero(kek, (word32)kekKeySz); XFREE(kek, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(cek, pkcs7->heap, DYNAMIC_TYPE_PKCS7); return ASN_PARSE_E; @@ -11803,7 +11817,9 @@ static int wc_PKCS7_DecryptPwri(wc_PKCS7* pkcs7, byte* in, word32 inSz, pwriEncAlgoId); if (ret < 0) { XFREE(salt, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + ForceZero(kek, (word32)kekKeySz); XFREE(kek, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + ForceZero(cek, cekSz); XFREE(cek, pkcs7->heap, DYNAMIC_TYPE_PKCS7); return ret; } @@ -11812,7 +11828,9 @@ static int wc_PKCS7_DecryptPwri(wc_PKCS7* pkcs7, byte* in, word32 inSz, if (*decryptedKeySz < cekSz) { WOLFSSL_MSG("Decrypted key buffer too small for CEK"); XFREE(salt, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + ForceZero(kek, (word32)kekKeySz); XFREE(kek, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + ForceZero(cek, cekSz); XFREE(cek, pkcs7->heap, DYNAMIC_TYPE_PKCS7); return BUFFER_E; } @@ -11821,7 +11839,9 @@ static int wc_PKCS7_DecryptPwri(wc_PKCS7* pkcs7, byte* in, word32 inSz, *decryptedKeySz = cekSz; XFREE(salt, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + ForceZero(kek, (word32)kekKeySz); XFREE(kek, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + ForceZero(cek, cekSz); XFREE(cek, pkcs7->heap, DYNAMIC_TYPE_PKCS7); /* mark recipFound, since we only support one RecipientInfo for now */ From 4e423fde170608e67db623b3d9b10f3a489e00ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 14 Apr 2026 13:51:28 +0200 Subject: [PATCH 06/11] More PKCS#7 bounds checks --- wolfcrypt/src/pkcs7.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index c27ce5e2b1..ec7fd317d2 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -7046,6 +7046,9 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf, idx += (word32)length; } + else if (ret == 0) { + ret = ASN_PARSE_E; + } pkcs7->content = content; pkcs7->contentSz = (word32)contentSz; @@ -9626,7 +9629,7 @@ static int wc_PKCS7_PwriKek_KeyUnWrap(wc_PKCS7* pkcs7, const byte* kek, cekLen = outTmp[0]; /* verify length */ - fail |= ctMaskGT(cekLen, (int)inSz); + fail |= ctMaskGT(cekLen, (int)inSz - 4); /* verify check bytes */ fail |= ctMaskNotEq((int)(outTmp[1] ^ outTmp[4]), 0xFF); fail |= ctMaskNotEq((int)(outTmp[2] ^ outTmp[5]), 0xFF); @@ -11933,7 +11936,9 @@ static int wc_PKCS7_DecryptKekri(wc_PKCS7* pkcs7, byte* in, word32 inSz, &datePtr, &dateFormat, &dateLen) != 0) { return ASN_PARSE_E; } - *idx += (word32)(dateLen + 1); + /* datePtr points to the start of the date value + * within pkiMsg; advance past the full TLV. */ + *idx = (word32)(datePtr - pkiMsg) + (word32)dateLen; } if (*idx > pkiMsgSz) { @@ -13102,6 +13107,14 @@ int wc_PKCS7_DecodeEnvelopedData(wc_PKCS7* pkcs7, byte* in, ret = ASN_PARSE_E; } + #ifdef NO_PKCS7_STREAM + if (ret == 0 && encryptedContentTotalSz > (int)(pkiMsgSz - idx)) { + /* In non-streaming mode, ensure the content fits in the buffer. + * Streaming mode handles this via AddDataToStream. */ + ret = BUFFER_E; + } + #endif + if (ret != 0) break; @@ -15355,6 +15368,12 @@ int wc_PKCS7_DecodeEncryptedData(wc_PKCS7* pkcs7, byte* in, word32 inSz, pkiMsgSz, NO_USER_CHECK) <= 0) ret = ASN_PARSE_E; +#ifdef NO_PKCS7_STREAM + if (ret == 0 && encryptedContentSz > (int)(pkiMsgSz - idx)) { + ret = BUFFER_E; + } +#endif + if (ret < 0) break; #ifndef NO_PKCS7_STREAM @@ -15392,7 +15411,8 @@ int wc_PKCS7_DecodeEncryptedData(wc_PKCS7* pkcs7, byte* in, word32 inSz, version = (int)pkcs7->stream->vers; tmpIv = pkcs7->stream->tmpIv; #endif - if (encryptedContentSz <= 0) { + if (encryptedContentSz <= 0 || + encryptedContentSz > (int)(pkiMsgSz - idx)) { ret = BUFFER_E; break; } From 3fd4060458baf3feb558bc935d2dc248aa62675a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 14 Apr 2026 14:23:17 +0200 Subject: [PATCH 07/11] Add more PKCS#7 tests --- tests/api.c | 605 +++++++++++++++++++++++++++++++++++++++++ tests/api/test_pkcs7.c | 5 + 2 files changed, 610 insertions(+) diff --git a/tests/api.c b/tests/api.c index 7e20480484..eca1ba38f1 100644 --- a/tests/api.c +++ b/tests/api.c @@ -35960,6 +35960,605 @@ static int test_pkcs7_ori_seqsz_underflow(void) return EXPECT_RESULT(); } +/* Test: PKCS#7 ORI must reject when seqSz extends oriValue past input buffer. + * + * The first ORI bounds check (OID exceeds SEQUENCE boundary) is covered by + * test_pkcs7_ori_seqsz_underflow. This test covers the *second* check: + * oriValue region extends past the end of the input buffer. We craft a + * message where the [4] SEQUENCE length is valid relative to the OID + * (OID fits inside it), but the remaining oriValue portion extends past + * the end of the actual input buffer. + * + * To bypass GetLength's own bounds validation, we set the outer lengths + * (EnvelopedData SEQUENCE, RecipientInfos SET) to match the [4] claim, + * but truncate the actual buffer we pass to DecodeEnvelopedData. */ +static int test_pkcs7_ori_orivalue_overflow(void) +{ + EXPECT_DECLS; +#if defined(HAVE_PKCS7) && !defined(WOLFSSL_NO_MALLOC) + wc_PKCS7* p7 = NULL; + byte out[256]; + + /* EnvelopedData with [4] ORI whose seqSz (40) is valid for the OID + * (6 bytes consumed) but oriValueSz (34) extends past the truncated + * input buffer. + * + * Layout: + * ContentInfo SEQUENCE + * OID envelopedData + * [0] EXPLICIT + * EnvelopedData SEQUENCE + * version = 0 + * RecipientInfos SET + * [4] CONSTRUCTED (seqSz = 40) + * OID (tag 06, len 04, 4 content bytes = 6 total) + * oriValue should be 34 bytes but input is truncated + * EncryptedContentInfo (filler for streaming) + * + * We pass the full array to DecodeEnvelopedData, but the actual + * input ends before [4]'s declared 40 bytes are consumed. + */ + static const byte poc[] = { + /* ContentInfo SEQUENCE */ + 0x30, 0x43, + /* contentType = envelopedData 1.2.840.113549.1.7.3 */ + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x03, + /* [0] EXPLICIT */ + 0xa0, 0x36, + /* EnvelopedData SEQUENCE */ + 0x30, 0x34, + /* version = 0 */ + 0x02, 0x01, 0x00, + /* RecipientInfos SET (len = 44 covers [4] tag+len+40) */ + 0x31, 0x2c, + /* [4] CONSTRUCTED = ORI implicit SEQUENCE, seqSz = 40 */ + 0xa4, 0x28, + /* OID: tag=06, len=04, 4 content bytes = 6 total */ + 0x06, 0x04, 0x2a, 0x03, 0x04, 0x05, + /* Only 4 bytes of oriValue here, but seqSz claims 34 more */ + 0x00, 0x00, 0x00, 0x00, + /* EncryptedContentInfo SEQUENCE (filler) */ + 0x30, 0x0b, + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, + 0x01 + }; + + p7 = wc_PKCS7_New(NULL, INVALID_DEVID); + ExpectNotNull(p7); + if (p7 != NULL) { + wc_PKCS7_SetOriDecryptCb(p7, test_dummy_ori_cb); + + /* Must return error - oriValue extends past input buffer */ + ExpectIntLT(wc_PKCS7_DecodeEnvelopedData(p7, (byte*)poc, sizeof(poc), + out, sizeof(out)), 0); + + wc_PKCS7_Free(p7); + } +#endif + return EXPECT_RESULT(); +} + +/* Test: PKCS#7 KTRI must not match recipient when SKID length differs + * from expected keyIdSize. + * + * The fix adds a `length == keyIdSize` check before comparing the SKID + * bytes. Without this check, XMEMCMP could compare against data beyond + * the SKID content. This test crafts a KTRI RecipientInfo where the + * [0] SubjectKeyIdentifier has length 5 instead of KEYID_SIZE (20). + * The decode must return an error (no matching recipient). */ +static int test_pkcs7_ktri_skid_length_mismatch(void) +{ + EXPECT_DECLS; +#if defined(HAVE_PKCS7) && !defined(NO_RSA) && !defined(WOLFSSL_NO_MALLOC) + wc_PKCS7* p7 = NULL; + byte out[256]; + + /* Minimal EnvelopedData with KTRI using version=2 (SKID path). + * The SKID [0] has length 5 instead of 20. + * + * ContentInfo SEQUENCE + * OID envelopedData + * [0] EXPLICIT + * EnvelopedData SEQUENCE + * version = 2 + * RecipientInfos SET + * KTRI SEQUENCE + * version = 2 + * [0] SKID (5 bytes, should be 20) + * AlgorithmIdentifier (RSA OID) + * OCTET STRING (fake encrypted key) + * EncryptedContentInfo (filler) + */ + static const byte poc[] = { + /* ContentInfo SEQUENCE */ + 0x30, 0x46, + /* contentType = envelopedData 1.2.840.113549.1.7.3 */ + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x03, + /* [0] EXPLICIT */ + 0xa0, 0x39, + /* EnvelopedData SEQUENCE */ + 0x30, 0x37, + /* version = 2 (triggers SKID-based recipient identification) */ + 0x02, 0x01, 0x02, + /* RecipientInfos SET */ + 0x31, 0x21, + /* KTRI SEQUENCE */ + 0x30, 0x1f, + /* version = 2 (SKID) */ + 0x02, 0x01, 0x02, + /* [0] IMPLICIT SubjectKeyIdentifier, length = 5 (wrong!) */ + 0x80, 0x05, 0x01, 0x02, 0x03, 0x04, 0x05, + /* AlgorithmIdentifier: RSA 1.2.840.113549.1.1.1 */ + 0x30, 0x0d, + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, + 0x01, 0x01, 0x01, + 0x05, 0x00, /* NULL params */ + /* encryptedKey OCTET STRING (2 bytes fake) */ + 0x04, 0x02, 0xAA, 0xBB, + /* EncryptedContentInfo SEQUENCE (filler) */ + 0x30, 0x0b, + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, + 0x01 + }; + + p7 = wc_PKCS7_New(NULL, INVALID_DEVID); + ExpectNotNull(p7); + if (p7 != NULL) { + /* Decode without a cert - SKID will never match, and the + * mismatched SKID length must not cause out-of-bounds reads */ + ExpectIntLT(wc_PKCS7_DecodeEnvelopedData(p7, (byte*)poc, sizeof(poc), + out, sizeof(out)), 0); + + wc_PKCS7_Free(p7); + } +#endif + return EXPECT_RESULT(); +} + +/* Test: PKCS#7 KARI must reject BIT STRING with length < 2 in + * OriginatorPublicKey. + * + * The fix adds `if (length < 2) return ASN_PARSE_E` after parsing + * the BIT STRING tag and length. A BIT STRING must have at least + * the unused-bits byte plus one byte of content. This test crafts + * a KARI [1] RecipientInfo where the OriginatorPublicKey's BIT STRING + * has length 1 (degenerate). The PKCS7 object is initialized with a + * real ECC cert so that KariParseRecipCert succeeds and parsing reaches + * the BIT STRING validation. */ +static int test_pkcs7_kari_degenerate_bitstring(void) +{ + EXPECT_DECLS; +#if defined(HAVE_PKCS7) && defined(HAVE_ECC) && defined(HAVE_X963_KDF) && \ + !defined(WOLFSSL_NO_MALLOC) + wc_PKCS7* p7 = NULL; + byte out[256]; + byte* eccCert = NULL; + byte* eccPrivKey = NULL; + word32 eccCertSz = 0; + word32 eccPrivKeySz = 0; +#if !defined(USE_CERT_BUFFERS_256) && !defined(NO_FILESYSTEM) + XFILE f = XBADFILE; +#endif + + /* Minimal EnvelopedData with KARI [1] containing a degenerate + * BIT STRING (length 1) in OriginatorPublicKey. + * + * ContentInfo SEQUENCE + * OID envelopedData + * [0] EXPLICIT + * EnvelopedData SEQUENCE + * version = 2 + * RecipientInfos SET + * [1] CONSTRUCTED (KARI) + * version = 3 + * [0] CONSTRUCTED (OriginatorIdentifierOrKey) + * [1] CONSTRUCTED (OriginatorPublicKey) + * AlgorithmIdentifier (ECDSAk) + * BIT STRING length=1 (degenerate!) + * EncryptedContentInfo (filler) + */ + static const byte poc[] = { + /* ContentInfo SEQUENCE */ + 0x30, 0x44, + /* contentType = envelopedData 1.2.840.113549.1.7.3 */ + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x03, + /* [0] EXPLICIT */ + 0xa0, 0x37, + /* EnvelopedData SEQUENCE */ + 0x30, 0x35, + /* version = 2 */ + 0x02, 0x01, 0x02, + /* RecipientInfos SET */ + 0x31, 0x1f, + /* [1] CONSTRUCTED (KARI implicit) */ + 0xa1, 0x1d, + /* version = 3 */ + 0x02, 0x01, 0x03, + /* [0] CONSTRUCTED (OriginatorIdentifierOrKey) */ + 0xa0, 0x18, + /* [1] CONSTRUCTED (OriginatorPublicKey) */ + 0xa1, 0x16, + /* AlgorithmIdentifier SEQUENCE */ + 0x30, 0x13, + /* OID: id-ecPublicKey 1.2.840.10045.2.1 */ + 0x06, 0x07, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x02, 0x01, + /* OID: prime256v1 1.2.840.10045.3.1.7 */ + 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x03, + 0x01, 0x07, + /* BIT STRING with length 1 - degenerate! */ + 0x03, 0x01, 0x00, + /* EncryptedContentInfo SEQUENCE (filler) */ + 0x30, 0x0b, + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, + 0x01 + }; + + /* Load ECC cert and key so KariParseRecipCert succeeds and + * parsing reaches the BIT STRING check */ +#ifdef USE_CERT_BUFFERS_256 + eccCertSz = (word32)sizeof_cliecc_cert_der_256; + ExpectNotNull(eccCert = (byte*)XMALLOC(eccCertSz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + if (eccCert != NULL) + XMEMCPY(eccCert, cliecc_cert_der_256, eccCertSz); + eccPrivKeySz = (word32)sizeof_ecc_clikey_der_256; + ExpectNotNull(eccPrivKey = (byte*)XMALLOC(eccPrivKeySz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + if (eccPrivKey != NULL) + XMEMCPY(eccPrivKey, ecc_clikey_der_256, eccPrivKeySz); +#elif !defined(NO_FILESYSTEM) + eccCertSz = FOURK_BUF; + ExpectNotNull(eccCert = (byte*)XMALLOC(eccCertSz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + ExpectTrue((f = XFOPEN("./certs/client-ecc-cert.der", "rb")) != XBADFILE); + ExpectTrue((eccCertSz = (word32)XFREAD(eccCert, 1, eccCertSz, f)) > 0); + if (f != XBADFILE) { + XFCLOSE(f); + f = XBADFILE; + } + eccPrivKeySz = FOURK_BUF; + ExpectNotNull(eccPrivKey = (byte*)XMALLOC(eccPrivKeySz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + ExpectTrue((f = XFOPEN("./certs/ecc-client-key.der", "rb")) != XBADFILE); + ExpectTrue((eccPrivKeySz = (word32)XFREAD(eccPrivKey, 1, eccPrivKeySz, + f)) > 0); + if (f != XBADFILE) + XFCLOSE(f); +#else + eccCert = NULL; + eccCertSz = 0; + eccPrivKey = NULL; + eccPrivKeySz = 0; +#endif + + p7 = wc_PKCS7_New(HEAP_HINT, INVALID_DEVID); + ExpectNotNull(p7); + if (p7 != NULL && eccCert != NULL) { + ExpectIntEQ(wc_PKCS7_InitWithCert(p7, eccCert, eccCertSz), 0); + if (p7 != NULL) { + p7->privateKey = eccPrivKey; + p7->privateKeySz = eccPrivKeySz; + } + + /* Must return error - BIT STRING length < 2 is invalid */ + ExpectIntLT(wc_PKCS7_DecodeEnvelopedData(p7, (byte*)poc, sizeof(poc), + out, sizeof(out)), 0); + + if (p7 != NULL) { + p7->privateKey = NULL; + p7->privateKeySz = 0; + } + wc_PKCS7_Free(p7); + } + else if (p7 != NULL) { + wc_PKCS7_Free(p7); + } + + XFREE(eccCert, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(eccPrivKey, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); +#endif + return EXPECT_RESULT(); +} + +/* Test: PKCS#7 EncryptedData must reject when encryptedContentSz exceeds + * the remaining input buffer. + * + * The fix adds `encryptedContentSz > (int)(pkiMsgSz - idx)` to the + * existing `encryptedContentSz <= 0` check in stage 6. This test crafts + * a minimal EncryptedData where the [0] IMPLICIT content length claims + * 0x200 (512) bytes but only 32 bytes of ciphertext are present. */ +static int test_pkcs7_encrypted_content_size_overflow(void) +{ + EXPECT_DECLS; +#if defined(HAVE_PKCS7) && !defined(NO_AES) && defined(HAVE_AES_CBC) && \ + defined(WOLFSSL_AES_256) && !defined(NO_PKCS7_ENCRYPTED_DATA) && \ + !defined(WOLFSSL_NO_MALLOC) + wc_PKCS7* p7 = NULL; + byte key[32]; + byte out[256]; + + /* EncryptedData with [0] content claiming 512 bytes but only 32 present. + * + * ContentInfo SEQUENCE + * OID encryptedData (1.2.840.113549.1.7.6) + * [0] EXPLICIT + * EncryptedData SEQUENCE + * version = 0 + * EncryptedContentInfo SEQUENCE + * OID data (1.2.840.113549.1.7.1) + * AlgorithmIdentifier + * OID AES-256-CBC (2.16.840.1.101.3.4.1.42) + * OCTET STRING IV (16 zero bytes) + * [0] IMPLICIT content (claimed len=0x200, actual=32 bytes) + */ + static const byte poc[] = { + /* ContentInfo SEQUENCE (len covers entire message) */ + 0x30, 0x50, + /* OID encryptedData 1.2.840.113549.1.7.6 */ + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x06, + /* [0] EXPLICIT */ + 0xa0, 0x43, + /* EncryptedData SEQUENCE */ + 0x30, 0x41, + /* version = 0 */ + 0x02, 0x01, 0x00, + /* EncryptedContentInfo SEQUENCE */ + 0x30, 0x3c, + /* OID data 1.2.840.113549.1.7.1 */ + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, + 0x01, + /* AlgorithmIdentifier SEQUENCE */ + 0x30, 0x1d, + /* OID AES-256-CBC 2.16.840.1.101.3.4.1.42 */ + 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x01, + 0x2a, + /* IV: OCTET STRING (16 zero bytes) */ + 0x04, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + /* [0] IMPLICIT encryptedContent - claims 512 bytes! + * Only 16 bytes of fake ciphertext follow. */ + 0x80, 0x82, 0x02, 0x00, + 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, + 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA + }; + + XMEMSET(key, 0, sizeof(key)); + + p7 = wc_PKCS7_New(NULL, INVALID_DEVID); + ExpectNotNull(p7); + if (p7 != NULL) { + p7->encryptionKey = key; + p7->encryptionKeySz = sizeof(key); + + /* Must return error - content extends past input buffer */ + ExpectIntLT(wc_PKCS7_DecodeEncryptedData(p7, (byte*)poc, sizeof(poc), + out, sizeof(out)), 0); + + wc_PKCS7_Free(p7); + } +#endif + return EXPECT_RESULT(); +} + +/* Test: PKCS#7 SignedData must reject when the signature field is not + * an OCTET STRING. + * + * The fix adds `else if (ret == 0) { ret = ASN_PARSE_E; }` so that + * when the tag at the signature position is not ASN_OCTET_STRING, + * parsing returns an error instead of silently continuing with no + * signature. This test encodes a valid SignedData, then corrupts + * the signature OCTET STRING tag and verifies that decode fails. */ +static int test_pkcs7_signed_bad_sig_tag(void) +{ + EXPECT_DECLS; +#if defined(HAVE_PKCS7) && !defined(NO_RSA) && !defined(NO_SHA256) && \ + !defined(WOLFSSL_NO_MALLOC) + PKCS7* pkcs7 = NULL; + WC_RNG rng; + byte encoded[FOURK_BUF]; + int encodedSz = 0; + int i; + byte* rsaCert = NULL; + byte* rsaPrivKey = NULL; + word32 rsaCertSz = 0; + word32 rsaPrivKeySz = 0; +#if !defined(USE_CERT_BUFFERS_2048) && !defined(USE_CERT_BUFFERS_1024) && \ + !defined(NO_FILESYSTEM) + XFILE f = XBADFILE; +#endif + + const byte data[] = "Test signed data"; + + XMEMSET(&rng, 0, sizeof(WC_RNG)); + + /* Load RSA cert and key */ +#if defined(USE_CERT_BUFFERS_2048) + rsaCertSz = (word32)sizeof_client_cert_der_2048; + ExpectNotNull(rsaCert = (byte*)XMALLOC(rsaCertSz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + if (rsaCert != NULL) + XMEMCPY(rsaCert, client_cert_der_2048, rsaCertSz); + rsaPrivKeySz = (word32)sizeof_client_key_der_2048; + ExpectNotNull(rsaPrivKey = (byte*)XMALLOC(rsaPrivKeySz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + if (rsaPrivKey != NULL) + XMEMCPY(rsaPrivKey, client_key_der_2048, rsaPrivKeySz); +#elif defined(USE_CERT_BUFFERS_1024) + rsaCertSz = (word32)sizeof_client_cert_der_1024; + ExpectNotNull(rsaCert = (byte*)XMALLOC(rsaCertSz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + if (rsaCert != NULL) + XMEMCPY(rsaCert, client_cert_der_1024, rsaCertSz); + rsaPrivKeySz = (word32)sizeof_client_key_der_1024; + ExpectNotNull(rsaPrivKey = (byte*)XMALLOC(rsaPrivKeySz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + if (rsaPrivKey != NULL) + XMEMCPY(rsaPrivKey, client_key_der_1024, rsaPrivKeySz); +#elif !defined(NO_FILESYSTEM) + rsaCertSz = FOURK_BUF; + ExpectNotNull(rsaCert = (byte*)XMALLOC(rsaCertSz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + ExpectTrue((f = XFOPEN("./certs/client-cert.der", "rb")) != XBADFILE); + ExpectTrue((rsaCertSz = (word32)XFREAD(rsaCert, 1, rsaCertSz, f)) > 0); + if (f != XBADFILE) { XFCLOSE(f); f = XBADFILE; } + rsaPrivKeySz = FOURK_BUF; + ExpectNotNull(rsaPrivKey = (byte*)XMALLOC(rsaPrivKeySz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + ExpectTrue((f = XFOPEN("./certs/client-key.der", "rb")) != XBADFILE); + ExpectTrue((rsaPrivKeySz = (word32)XFREAD(rsaPrivKey, 1, + rsaPrivKeySz, f)) > 0); + if (f != XBADFILE) XFCLOSE(f); +#else + rsaCert = NULL; rsaCertSz = 0; + rsaPrivKey = NULL; rsaPrivKeySz = 0; +#endif + + if (rsaCert == NULL || rsaPrivKey == NULL) { + XFREE(rsaCert, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(rsaPrivKey, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + return TEST_SKIPPED; + } + + ExpectIntEQ(wc_InitRng(&rng), 0); + + /* Encode a valid SignedData */ + ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); + ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, rsaCert, rsaCertSz), 0); + if (pkcs7 != NULL) { + pkcs7->content = (byte*)data; + pkcs7->contentSz = (word32)sizeof(data); + pkcs7->contentOID = DATA; + pkcs7->hashOID = SHA256h; + pkcs7->encryptOID = RSAk; + pkcs7->privateKey = rsaPrivKey; + pkcs7->privateKeySz = rsaPrivKeySz; + pkcs7->rng = &rng; + } + + ExpectIntGT(encodedSz = wc_PKCS7_EncodeSignedData(pkcs7, encoded, + sizeof(encoded)), 0); + wc_PKCS7_Free(pkcs7); + pkcs7 = NULL; + + /* Find the signature OCTET STRING tag (0x04) near the end of the + * encoded message and corrupt it. The signature is the last large + * OCTET STRING in the SignerInfo. Search backwards for 0x04 followed + * by a length that looks like an RSA signature (>= 64 bytes). + * This heuristic depends on the signature being the last large + * OCTET STRING; the found==1 assertion below guards against + * silent false passes if encoding changes. */ + if (EXPECT_SUCCESS()) { + int found = 0; + for (i = encodedSz - 10; i > 10; i--) { + if (encoded[i] == 0x04) { + int len = 0, lbytes = 0; + if (encoded[i+1] < 0x80) { + len = encoded[i+1]; lbytes = 1; + } + else if (encoded[i+1] == 0x81) { + len = encoded[i+2]; lbytes = 2; + } + else if (encoded[i+1] == 0x82) { + len = (encoded[i+2] << 8) | encoded[i+3]; lbytes = 3; + } + /* RSA signature is typically >= 128 bytes */ + if (len >= 64 && i + 1 + lbytes + len <= encodedSz) { + /* Corrupt the OCTET STRING tag to INTEGER */ + encoded[i] = 0x02; /* ASN_INTEGER instead of OCTET STRING */ + found = 1; + break; + } + } + } + ExpectIntEQ(found, 1); + } + + /* Verify the corrupted SignedData - must fail */ + if (EXPECT_SUCCESS()) { + ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); + ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); + ExpectIntLT(wc_PKCS7_VerifySignedData(pkcs7, encoded, + (word32)encodedSz), 0); + wc_PKCS7_Free(pkcs7); + pkcs7 = NULL; + } + + DoExpectIntEQ(wc_FreeRng(&rng), 0); + XFREE(rsaCert, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(rsaPrivKey, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); +#endif + return EXPECT_RESULT(); +} + +/* Test: PKCS#7 EnvelopedData must reject when encryptedContentTotalSz + * exceeds the remaining input buffer. + * + * The fix adds a bounds check under NO_PKCS7_STREAM, but the same + * crafted message should also be properly handled in streaming mode. + * This test crafts an EnvelopedData where the [0] content length + * claims 512 bytes but only minimal data follows. */ +static int test_pkcs7_enveloped_content_size_overflow(void) +{ + EXPECT_DECLS; +#if defined(HAVE_PKCS7) && !defined(WOLFSSL_NO_MALLOC) && !defined(NO_RSA) + wc_PKCS7* p7 = NULL; + byte out[256]; + + /* EnvelopedData with KTRI where the EncryptedContentInfo [0] claims + * 512 bytes but only 16 are present. + * + * The outer structure is valid enough to reach the content parsing: + * ContentInfo -> EnvelopedData -> version=0 -> + * RecipientInfos (empty SET) -> EncryptedContentInfo -> + * contentType + AlgorithmIdentifier + [0] oversized content */ + static const byte poc[] = { + /* ContentInfo SEQUENCE */ + 0x30, 0x50, + /* OID envelopedData 1.2.840.113549.1.7.3 */ + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x03, + /* [0] EXPLICIT */ + 0xa0, 0x43, + /* EnvelopedData SEQUENCE */ + 0x30, 0x41, + /* version = 0 */ + 0x02, 0x01, 0x00, + /* RecipientInfos SET (empty - no recipients) */ + 0x31, 0x00, + /* EncryptedContentInfo SEQUENCE */ + 0x30, 0x3c, + /* OID data 1.2.840.113549.1.7.1 */ + 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x07, + 0x01, + /* AlgorithmIdentifier SEQUENCE */ + 0x30, 0x1d, + /* OID AES-256-CBC 2.16.840.1.101.3.4.1.42 */ + 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x01, + 0x2a, + /* IV: OCTET STRING (16 zero bytes) */ + 0x04, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + /* [0] IMPLICIT encryptedContent - claims 512 bytes! */ + 0x80, 0x82, 0x02, 0x00, + 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, + 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA, 0xAA + }; + + p7 = wc_PKCS7_New(NULL, INVALID_DEVID); + ExpectNotNull(p7); + if (p7 != NULL) { + /* With an empty RecipientInfos SET, the function may fail at + * the recipient matching stage before reaching the content-size + * bounds check. The ExpectIntLT assertion ensures the + * oversized content does not cause a buffer over-read. */ + ExpectIntLT(wc_PKCS7_DecodeEnvelopedData(p7, (byte*)poc, sizeof(poc), + out, sizeof(out)), 0); + + wc_PKCS7_Free(p7); + } +#endif + return EXPECT_RESULT(); +} + /* Dilithium verify_ctx_msg must reject absurdly large msgLen */ static int test_dilithium_hash(void) { @@ -36902,6 +37501,12 @@ TEST_CASE testCases[] = { TEST_DECL(test_pkcs7_decode_encrypted_outputsz), TEST_DECL(test_pkcs7_ori_oversized_oid), TEST_DECL(test_pkcs7_ori_seqsz_underflow), + TEST_DECL(test_pkcs7_ori_orivalue_overflow), + TEST_DECL(test_pkcs7_ktri_skid_length_mismatch), + TEST_DECL(test_pkcs7_kari_degenerate_bitstring), + TEST_DECL(test_pkcs7_encrypted_content_size_overflow), + TEST_DECL(test_pkcs7_signed_bad_sig_tag), + TEST_DECL(test_pkcs7_enveloped_content_size_overflow), TEST_DECL(test_pkcs7_padding), #if defined(WOLFSSL_SNIFFER) && defined(WOLFSSL_SNIFFER_CHAIN_INPUT) diff --git a/tests/api/test_pkcs7.c b/tests/api/test_pkcs7.c index cd9f7aa575..646db1bdb8 100644 --- a/tests/api/test_pkcs7.c +++ b/tests/api/test_pkcs7.c @@ -2763,6 +2763,11 @@ int test_wc_PKCS7_EncodeDecodeEnvelopedData(void) AES128CBCb, AES128_WRAP, dhSinglePass_stdDH_sha1kdf_scheme, eccCert, eccCertSz, eccPrivKey, eccPrivKeySz}, #endif + #if defined(WOLFSSL_SHA224) && defined(WOLFSSL_AES_128) + {(byte*)input, (word32)(sizeof(input)/sizeof(char)), DATA, + AES128CBCb, AES128_WRAP, dhSinglePass_stdDH_sha224kdf_scheme, + eccCert, eccCertSz, eccPrivKey, eccPrivKeySz}, + #endif #if !defined(NO_SHA256) && defined(WOLFSSL_AES_256) {(byte*)input, (word32)(sizeof(input)/sizeof(char)), DATA, AES256CBCb, AES256_WRAP, dhSinglePass_stdDH_sha256kdf_scheme, From 589feabc0cb13a1747139cb1fdef5b6d26ed4baf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 14 Apr 2026 18:04:12 +0200 Subject: [PATCH 08/11] Harden PKCS#7 EnvelopedData key unwrap --- .wolfssl_known_macro_extras | 1 + tests/api/test_pkcs7.c | 251 +++++++++++++++++++++++++++++++++++- tests/api/test_pkcs7.h | 21 +++ wolfcrypt/src/pkcs7.c | 250 ++++++++++++++++++++++++++++++++++- 4 files changed, 510 insertions(+), 13 deletions(-) diff --git a/.wolfssl_known_macro_extras b/.wolfssl_known_macro_extras index 3943d47739..cd06d68a1f 100644 --- a/.wolfssl_known_macro_extras +++ b/.wolfssl_known_macro_extras @@ -829,6 +829,7 @@ WOLFSSL_NO_KCAPI_HMAC_SHA256 WOLFSSL_NO_KCAPI_HMAC_SHA384 WOLFSSL_NO_KCAPI_HMAC_SHA512 WOLFSSL_NO_KCAPI_SHA224 +WOLFSSL_NO_KTRI_ORACLE_WARNING WOLFSSL_NO_OCSP_DATE_CHECK WOLFSSL_NO_OCSP_ISSUER_CHAIN_CHECK WOLFSSL_NO_OCSP_OPTIONAL_CERTS diff --git a/tests/api/test_pkcs7.c b/tests/api/test_pkcs7.c index 646db1bdb8..a22fd9e10e 100644 --- a/tests/api/test_pkcs7.c +++ b/tests/api/test_pkcs7.c @@ -1118,6 +1118,229 @@ int test_wc_PKCS7_EnvelopedData_KTRI_RSA_PSS(void) #endif +/* + * Bleichenbacher padding-oracle regression: wc_PKCS7_DecryptKtri must not + * return a distinguishable error when RSA PKCS#1 v1.5 unwrap of the + * encrypted CEK fails vs. when it succeeds with a wrong key. The + * mitigation substitutes a deterministic pseudo-random CEK on RSA failure + * so content decryption fails indistinguishably. This test corrupts the + * encryptedKey in a valid EnvelopedData and asserts the error matches + * content corruption rather than surfacing an RSA/recipient-level code. + * Runs for AES-128 and AES-256 because the fake CEK is a fixed 32 bytes: + * AES-128 (key size 16) exercises the path where the fake size differs + * from the real CEK size. + */ +#if defined(HAVE_PKCS7) && !defined(NO_RSA) && !defined(NO_SHA256) && \ + !defined(NO_AES) && defined(HAVE_AES_CBC) && defined(WOLFSSL_AES_128) && \ + defined(WOLFSSL_AES_256) && !defined(NO_HMAC) && \ + !defined(WOLFSSL_NO_MALLOC) && \ + (defined(USE_CERT_BUFFERS_2048) || defined(USE_CERT_BUFFERS_1024) || \ + !defined(NO_FILESYSTEM)) +static int pkcs7_ktri_bad_pad_case(int encryptOID, byte* rsaCert, + word32 rsaCertSz, byte* rsaPrivKey, + word32 rsaPrivKeySz, byte* encrypted, + word32 encryptedCap, byte* decoded, + word32 decodedCap) +{ + EXPECT_DECLS; + PKCS7* pkcs7 = NULL; + byte data[] = "PKCS7 KTRI bad-RSA-padding regression payload."; + int encryptedSz = 0; + int badKeyRet = 0; + int badContentRet = 0; + byte savedKeyByte = 0; + byte savedContentByte = 0; + word32 i; + word32 encryptedKeyOff = 0; + static const byte rsaEncOid[] = { + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, + 0x01, 0x01, 0x01 + }; + + ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); + ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, rsaCert, rsaCertSz), 0); + if (pkcs7 != NULL) { + pkcs7->content = data; + pkcs7->contentSz = (word32)sizeof(data); + pkcs7->contentOID = DATA; + pkcs7->encryptOID = encryptOID; + } + ExpectIntGT(encryptedSz = wc_PKCS7_EncodeEnvelopedData(pkcs7, + encrypted, encryptedCap), 0); + wc_PKCS7_Free(pkcs7); + pkcs7 = NULL; + + /* Locate the KTRI encryptedKey OCTET STRING. After the rsaEncryption + * OID there are NULL algorithm parameters (05 00), then a 256-byte + * OCTET STRING (tag 04, long-form length 82 01 00 for RSA-2048). */ + for (i = 0; (int)(i + sizeof(rsaEncOid)) < encryptedSz; i++) { + if (XMEMCMP(&encrypted[i], rsaEncOid, sizeof(rsaEncOid)) == 0) { + word32 p = i + (word32)sizeof(rsaEncOid); + if (p + 2 < (word32)encryptedSz && + encrypted[p] == 0x05 && encrypted[p + 1] == 0x00) { + p += 2; + } + if (p + 4 < (word32)encryptedSz && encrypted[p] == 0x04) { + if (encrypted[p + 1] == 0x82) { + encryptedKeyOff = p + 4; + } + else if (encrypted[p + 1] == 0x81) { + encryptedKeyOff = p + 3; + } + else { + encryptedKeyOff = p + 2; + } + } + break; + } + } + ExpectIntGT(encryptedKeyOff, 0); + ExpectIntLT(encryptedKeyOff + 32, (word32)encryptedSz); + + /* Case 1: corrupt a byte inside the RSA ciphertext, decode, restore. */ + savedKeyByte = encrypted[encryptedKeyOff + 16]; + encrypted[encryptedKeyOff + 16] ^= 0xA5; + + ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); + ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, rsaCert, rsaCertSz), 0); + if (pkcs7 != NULL) { + pkcs7->privateKey = rsaPrivKey; + pkcs7->privateKeySz = rsaPrivKeySz; + } + badKeyRet = wc_PKCS7_DecodeEnvelopedData(pkcs7, encrypted, + (word32)encryptedSz, decoded, decodedCap); + wc_PKCS7_Free(pkcs7); + pkcs7 = NULL; + encrypted[encryptedKeyOff + 16] = savedKeyByte; + + /* Case 2: corrupt a byte in the second-to-last AES ciphertext block. + * In CBC mode this deterministically XOR-flips the corresponding byte + * in the last plaintext block, invalidating the PKCS#7 padding + * (original pad byte 0x01 becomes 0x01^0xA5 = 0xA4 > blockSz). + * Corrupting the last ciphertext block directly would randomize the + * entire last plaintext block, giving ~1/256 chance of accidentally + * valid padding and intermittent test failures. */ + savedContentByte = encrypted[encryptedSz - 17]; + encrypted[encryptedSz - 17] ^= 0xA5; + + ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); + ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, rsaCert, rsaCertSz), 0); + if (pkcs7 != NULL) { + pkcs7->privateKey = rsaPrivKey; + pkcs7->privateKeySz = rsaPrivKeySz; + } + badContentRet = wc_PKCS7_DecodeEnvelopedData(pkcs7, encrypted, + (word32)encryptedSz, decoded, decodedCap); + wc_PKCS7_Free(pkcs7); + pkcs7 = NULL; + encrypted[encryptedSz - 17] = savedContentByte; + + /* Case 2 must always fail: the CBC-chain corruption deterministically + * invalidates the PKCS#7 padding. */ + ExpectIntLT(badContentRet, 0); + /* Bad-key must NOT leak as an RSA- or recipient-level error. */ + ExpectIntNE(badKeyRet, WC_NO_ERR_TRACE(PKCS7_RECIP_E)); + ExpectIntNE(badKeyRet, WC_NO_ERR_TRACE(RSA_PAD_E)); + ExpectIntNE(badKeyRet, WC_NO_ERR_TRACE(RSA_BUFFER_E)); + ExpectIntNE(badKeyRet, WC_NO_ERR_TRACE(BAD_PADDING_E)); + /* Case 1 (bad RSA key) decrypts content with a random fake CEK, + * producing fully random plaintext. With ~1/256 probability the + * PKCS#7 padding accidentally looks valid, causing a positive + * garbage-length return instead of an error. This does not leak + * RSA key information, so it is acceptable. When both cases do + * fail, verify they fail at the same content-decryption layer. */ + if (badKeyRet < 0) { + ExpectIntEQ(badKeyRet, badContentRet); + } + + return EXPECT_RESULT(); +} + +int test_wc_PKCS7_EnvelopedData_KTRI_BadRsaPad(void) +{ + EXPECT_DECLS; + byte encrypted[FOURK_BUF]; + byte decoded[FOURK_BUF]; + byte* rsaCert = NULL; + byte* rsaPrivKey = NULL; + word32 rsaCertSz = 0; + word32 rsaPrivKeySz = 0; +#if !defined(USE_CERT_BUFFERS_1024) && !defined(USE_CERT_BUFFERS_2048) && \ + !defined(NO_FILESYSTEM) + XFILE f = XBADFILE; +#endif + + /* Load RSA cert and key */ +#if defined(USE_CERT_BUFFERS_1024) + rsaCertSz = (word32)sizeof_client_cert_der_1024; + ExpectNotNull(rsaCert = (byte*)XMALLOC(rsaCertSz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + if (rsaCert != NULL) + XMEMCPY(rsaCert, client_cert_der_1024, rsaCertSz); + rsaPrivKeySz = (word32)sizeof_client_key_der_1024; + ExpectNotNull(rsaPrivKey = (byte*)XMALLOC(rsaPrivKeySz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + if (rsaPrivKey != NULL) + XMEMCPY(rsaPrivKey, client_key_der_1024, rsaPrivKeySz); +#elif defined(USE_CERT_BUFFERS_2048) + rsaCertSz = (word32)sizeof_client_cert_der_2048; + ExpectNotNull(rsaCert = (byte*)XMALLOC(rsaCertSz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + if (rsaCert != NULL) + XMEMCPY(rsaCert, client_cert_der_2048, rsaCertSz); + rsaPrivKeySz = (word32)sizeof_client_key_der_2048; + ExpectNotNull(rsaPrivKey = (byte*)XMALLOC(rsaPrivKeySz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + if (rsaPrivKey != NULL) + XMEMCPY(rsaPrivKey, client_key_der_2048, rsaPrivKeySz); +#elif !defined(NO_FILESYSTEM) + rsaCertSz = FOURK_BUF; + ExpectNotNull(rsaCert = (byte*)XMALLOC(rsaCertSz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + ExpectTrue((f = XFOPEN("./certs/client-cert.der", "rb")) != XBADFILE); + ExpectTrue((rsaCertSz = (word32)XFREAD(rsaCert, 1, rsaCertSz, f)) > 0); + if (f != XBADFILE) { + XFCLOSE(f); + f = XBADFILE; + } + rsaPrivKeySz = FOURK_BUF; + ExpectNotNull(rsaPrivKey = (byte*)XMALLOC(rsaPrivKeySz, HEAP_HINT, + DYNAMIC_TYPE_TMP_BUFFER)); + ExpectTrue((f = XFOPEN("./certs/client-key.der", "rb")) != XBADFILE); + ExpectTrue((rsaPrivKeySz = (word32)XFREAD(rsaPrivKey, 1, + rsaPrivKeySz, f)) > 0); + if (f != XBADFILE) + XFCLOSE(f); +#endif + + if (rsaCert == NULL || rsaPrivKey == NULL) { + XFREE(rsaCert, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(rsaPrivKey, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + return TEST_SKIPPED; + } + + /* AES-128: 32-byte fake CEK larger than real CEK size (16 bytes). */ + ExpectIntEQ(pkcs7_ktri_bad_pad_case(AES128CBCb, rsaCert, rsaCertSz, + rsaPrivKey, rsaPrivKeySz, encrypted, sizeof(encrypted), + decoded, sizeof(decoded)), TEST_SUCCESS); +#ifdef WOLFSSL_AES_192 + /* AES-192: fake CEK (32) vs real CEK (24) - another size mismatch. */ + ExpectIntEQ(pkcs7_ktri_bad_pad_case(AES192CBCb, rsaCert, rsaCertSz, + rsaPrivKey, rsaPrivKeySz, encrypted, sizeof(encrypted), + decoded, sizeof(decoded)), TEST_SUCCESS); +#endif + /* AES-256: fake CEK size matches real CEK size (32 bytes). */ + ExpectIntEQ(pkcs7_ktri_bad_pad_case(AES256CBCb, rsaCert, rsaCertSz, + rsaPrivKey, rsaPrivKeySz, encrypted, sizeof(encrypted), + decoded, sizeof(decoded)), TEST_SUCCESS); + + XFREE(rsaCert, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(rsaPrivKey, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); + return EXPECT_RESULT(); +} /* END test_wc_PKCS7_EnvelopedData_KTRI_BadRsaPad */ +#endif + + /* * Testing wc_PKCS7_EncodeSignedData_ex() and wc_PKCS7_VerifySignedData_ex() */ @@ -2503,6 +2726,8 @@ int test_wc_PKCS7_DecodeEnvelopedData_multiple_recipients(void) bytes */ size_t testDerBufferSz = 0; byte decodedData[8192]; + byte serverDecodedData[8192]; + int serverRet = 0; ExpectTrue((f = XFOPEN(testFile, "rb")) != XBADFILE); if (f != XBADFILE) { @@ -2522,12 +2747,13 @@ int test_wc_PKCS7_DecodeEnvelopedData_multiple_recipients(void) ExpectIntEQ(wc_PKCS7_SetKey(pkcs7, (byte*)server_key_der_2048, sizeof_server_key_der_2048), 0); - ret = wc_PKCS7_DecodeEnvelopedData(pkcs7, testDerBuffer, - (word32)testDerBufferSz, decodedData, sizeof(decodedData)); + serverRet = wc_PKCS7_DecodeEnvelopedData(pkcs7, testDerBuffer, + (word32)testDerBufferSz, serverDecodedData, + sizeof(serverDecodedData)); #if defined(NO_AES) || defined(NO_AES_256) - ExpectIntEQ(ret, ALGO_ID_E); + ExpectIntEQ(serverRet, ALGO_ID_E); #else - ExpectIntGT(ret, 0); + ExpectIntGT(serverRet, 0); #endif wc_PKCS7_Free(pkcs7); pkcs7 = NULL; @@ -2553,7 +2779,14 @@ int test_wc_PKCS7_DecodeEnvelopedData_multiple_recipients(void) pkcs7 = NULL; } - /* test with ca cert recipient (which should fail) */ + /* Test with ca cert recipient. The ca cert is not a listed recipient, + * so RSA unwrap fails. The Bleichenbacher mitigation substitutes a + * pseudo-random fake CEK on unwrap failure, so the call normally + * returns a negative error when content decryption rejects the + * resulting garbage padding - but around 1/256 of the time the random + * CEK yields plaintext with accidentally-valid PKCS#7 padding and the + * call returns a non-negative "decrypted" size. That case must not + * produce the real plaintext. */ ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); if (pkcs7) { ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, (byte*)ca_cert_der_2048, @@ -2562,9 +2795,15 @@ int test_wc_PKCS7_DecodeEnvelopedData_multiple_recipients(void) ExpectIntEQ(wc_PKCS7_SetKey(pkcs7, (byte*)ca_key_der_2048, sizeof_ca_key_der_2048), 0); + XMEMSET(decodedData, 0, sizeof(decodedData)); ret = wc_PKCS7_DecodeEnvelopedData(pkcs7, testDerBuffer, (word32)testDerBufferSz, decodedData, sizeof(decodedData)); - ExpectIntLT(ret, 0); + #if defined(NO_AES) || defined(NO_AES_256) + ExpectIntEQ(ret, ALGO_ID_E); + #else + ExpectTrue(ret < 0 || ret != serverRet || + XMEMCMP(decodedData, serverDecodedData, (size_t)ret) != 0); + #endif wc_PKCS7_Free(pkcs7); pkcs7 = NULL; } diff --git a/tests/api/test_pkcs7.h b/tests/api/test_pkcs7.h index 084744d43c..e1c71c911d 100644 --- a/tests/api/test_pkcs7.h +++ b/tests/api/test_pkcs7.h @@ -38,6 +38,14 @@ int test_wc_PKCS7_EncodeSignedData_RSA_PSS(void); !defined(NO_AES) && defined(HAVE_AES_CBC) && defined(WOLFSSL_AES_256) int test_wc_PKCS7_EnvelopedData_KTRI_RSA_PSS(void); #endif +#if defined(HAVE_PKCS7) && !defined(NO_RSA) && !defined(NO_SHA256) && \ + !defined(NO_AES) && defined(HAVE_AES_CBC) && defined(WOLFSSL_AES_128) && \ + defined(WOLFSSL_AES_256) && !defined(NO_HMAC) && \ + !defined(WOLFSSL_NO_MALLOC) && \ + (defined(USE_CERT_BUFFERS_2048) || defined(USE_CERT_BUFFERS_1024) || \ + !defined(NO_FILESYSTEM)) +int test_wc_PKCS7_EnvelopedData_KTRI_BadRsaPad(void); +#endif int test_wc_PKCS7_EncodeSignedData_ex(void); int test_wc_PKCS7_VerifySignedData_RSA(void); int test_wc_PKCS7_VerifySignedData_ECC(void); @@ -82,6 +90,18 @@ int test_wc_PKCS7_VerifySignedData_IndefLenOOB(void); #define TEST_PKCS7_RSA_PSS_ED_DECL #endif +#if defined(HAVE_PKCS7) && !defined(NO_RSA) && !defined(NO_SHA256) && \ + !defined(NO_AES) && defined(HAVE_AES_CBC) && defined(WOLFSSL_AES_128) && \ + defined(WOLFSSL_AES_256) && !defined(NO_HMAC) && \ + !defined(WOLFSSL_NO_MALLOC) && \ + (defined(USE_CERT_BUFFERS_2048) || defined(USE_CERT_BUFFERS_1024) || \ + !defined(NO_FILESYSTEM)) +#define TEST_PKCS7_KTRI_BADRSAPAD_DECL \ + TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_EnvelopedData_KTRI_BadRsaPad), +#else +#define TEST_PKCS7_KTRI_BADRSAPAD_DECL +#endif + #define TEST_PKCS7_SIGNED_DATA_DECLS \ TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_InitWithCert), \ TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_EncodeData), \ @@ -100,6 +120,7 @@ int test_wc_PKCS7_VerifySignedData_IndefLenOOB(void); TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_DecodeEnvelopedData_stream), \ TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_EncodeDecodeEnvelopedData), \ TEST_PKCS7_RSA_PSS_ED_DECL \ + TEST_PKCS7_KTRI_BADRSAPAD_DECL \ TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_SetAESKeyWrapUnwrapCb), \ TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_GetEnvelopedDataKariRid), \ TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_EncodeEncryptedData), \ diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index ec7fd317d2..5d1a25bb31 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -44,6 +44,9 @@ #include #include +#ifndef NO_HMAC + #include +#endif #ifndef NO_RSA #include #endif @@ -207,6 +210,8 @@ static void wc_PKCS7_ResetStream(wc_PKCS7* pkcs7) #endif /* free any buffers that may be allocated */ + if (pkcs7->stream->aad != NULL && pkcs7->stream->aadSz > 0) + ForceZero(pkcs7->stream->aad, pkcs7->stream->aadSz); XFREE(pkcs7->stream->aad, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(pkcs7->stream->tag, pkcs7->heap, DYNAMIC_TYPE_PKCS7); XFREE(pkcs7->stream->nonce, pkcs7->heap, DYNAMIC_TYPE_PKCS7); @@ -7741,6 +7746,9 @@ static int wc_PKCS7_KariGenerateKEK(WC_PKCS7_KARI* kari, WC_RNG* rng, secret = (byte*)XMALLOC(secretSz, kari->heap, DYNAMIC_TYPE_PKCS7); if (secret == NULL) return MEMORY_E; +#ifdef WOLFSSL_CHECK_MEM_ZERO + wc_MemZero_Add("wc_PKCS7_KariGenerateKEK secret", secret, secretSz); +#endif #if defined(ECC_TIMING_RESISTANT) && (!defined(HAVE_FIPS) || \ (!defined(HAVE_FIPS_VERSION) || (HAVE_FIPS_VERSION != 2))) && \ @@ -10608,6 +10616,81 @@ int wc_PKCS7_EncodeEnvelopedData(wc_PKCS7* pkcs7, byte* output, word32 outputSz) } #ifndef NO_RSA +#if !defined(NO_HMAC) && !defined(NO_SHA256) +/* Bleichenbacher padding-oracle mitigation for PKCS#7/CMS KTRI: produce a + * WC_SHA256_DIGEST_SIZE-byte pseudo-random CEK derived from a fresh + * random seed and the encrypted-key ciphertext. The output is random per + * call (driven by the RNG seed); deriving via HMAC of the ciphertext + * simply gives the same value within one call regardless of where it is + * referenced. Called unconditionally so the work is in the timing path + * regardless of RSA padding validity. */ +static int wc_PKCS7_KtriFakeCEK(wc_PKCS7* pkcs7, const byte* encryptedKey, + word32 encryptedKeySz, byte* out) +{ + int ret; + byte seed[WC_SHA256_DIGEST_SIZE]; + WC_RNG* rng = NULL; + int ownRng = 0; + WC_DECLARE_VAR(localRng, WC_RNG, 1, pkcs7->heap); + WC_DECLARE_VAR(hmac, Hmac, 1, pkcs7->heap); + + if (pkcs7 == NULL || encryptedKey == NULL || out == NULL) { + return BAD_FUNC_ARG; + } + + WC_ALLOC_VAR_EX(hmac, Hmac, 1, pkcs7->heap, DYNAMIC_TYPE_HMAC, + return MEMORY_E); + + /* Prefer a caller-provided RNG to avoid paying a DRBG init/reseed cost + * on every decrypt (and to keep the timing envelope flatter on FIPS / + * HW-RNG builds). Fall back to a one-shot RNG when pkcs7->rng is not + * set. */ + if (pkcs7->rng != NULL) { + rng = pkcs7->rng; + } + else { + WC_ALLOC_VAR_EX(localRng, WC_RNG, 1, pkcs7->heap, DYNAMIC_TYPE_RNG, + WC_FREE_VAR_EX(hmac, pkcs7->heap, DYNAMIC_TYPE_HMAC); + return MEMORY_E); + ret = wc_InitRng_ex(localRng, pkcs7->heap, pkcs7->devId); + if (ret != 0) { + WC_FREE_VAR_EX(localRng, pkcs7->heap, DYNAMIC_TYPE_RNG); + WC_FREE_VAR_EX(hmac, pkcs7->heap, DYNAMIC_TYPE_HMAC); + return ret; + } + rng = localRng; + ownRng = 1; + } + + ret = wc_RNG_GenerateBlock(rng, seed, (word32)sizeof(seed)); + + if (ownRng) { + wc_FreeRng(localRng); + WC_FREE_VAR_EX(localRng, pkcs7->heap, DYNAMIC_TYPE_RNG); + } + + if (ret != 0) { + WC_FREE_VAR_EX(hmac, pkcs7->heap, DYNAMIC_TYPE_HMAC); + return ret; + } + + ret = wc_HmacInit(hmac, pkcs7->heap, pkcs7->devId); + if (ret == 0) { + ret = wc_HmacSetKey(hmac, WC_SHA256, seed, (word32)sizeof(seed)); + if (ret == 0) { + ret = wc_HmacUpdate(hmac, encryptedKey, encryptedKeySz); + } + if (ret == 0) { + ret = wc_HmacFinal(hmac, out); + } + wc_HmacFree(hmac); + } + ForceZero(seed, sizeof(seed)); + WC_FREE_VAR_EX(hmac, pkcs7->heap, DYNAMIC_TYPE_HMAC); + return ret; +} +#endif /* !NO_HMAC && !NO_SHA256 */ + /* decode KeyTransRecipientInfo (ktri), return 0 on success, <0 on error */ static int wc_PKCS7_DecryptKtri(wc_PKCS7* pkcs7, byte* in, word32 inSz, word32* idx, byte* decryptedKey, @@ -10967,25 +11050,146 @@ static int wc_PKCS7_DecryptKtri(wc_PKCS7* pkcs7, byte* in, word32 inSz, } wc_FreeRsaKey(privKey); + #if !defined(NO_HMAC) && !defined(NO_SHA256) + { + /* Bleichenbacher padding-oracle mitigation: always compute + * a pseudo-random fallback CEK so timing and error + * behaviour do not depend on RSA padding validity. On + * unwrap failure we substitute the fallback and let + * content decryption fail indistinguishably from "unwrap + * succeeded but CEK is wrong". */ + byte fakeKey[WC_SHA256_DIGEST_SIZE]; + int fakeRet = wc_PKCS7_KtriFakeCEK(pkcs7, encryptedKey, + (word32)encryptedKeySz, + fakeKey); + + if (fakeRet != 0) { + /* Fallback generation failed (e.g. RNG/HMAC error). + * Return the fallback-generation status, which does + * not depend on RSA padding validity, rather than the + * RSA status which would re-open the oracle. */ + ForceZero(fakeKey, sizeof(fakeKey)); + /* In the non-OAEP path RSA is decrypted in-place via + * wc_RsaPrivateDecryptInline, so encryptedKey holds + * the (possibly valid) plaintext CEK. Zero it before + * free. */ + ForceZero(encryptedKey, (word32)encryptedKeySz); + XFREE(encryptedKey, pkcs7->heap, DYNAMIC_TYPE_WOLF_BIGINT); + WC_FREE_VAR_EX(privKey, pkcs7->heap, + DYNAMIC_TYPE_TMP_BUFFER); + #ifndef WC_NO_RSA_OAEP + if (encOID == RSAESOAEPk) { + if (outKey != NULL) { + ForceZero(outKey, outKeySz); + XFREE(outKey, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + } + } + #endif + return fakeRet; + } + + /* Constant-time select between fake and real CEK. On RSA + * failure outKey may be NULL or keySz may be <= 0; in + * both cases the mask selects fakeKey for every byte. + * + * To avoid data-dependent branches that leak realLen, + * copy the real key into a fixed-size zero-padded buffer + * first, then select byte-by-byte in constant time. */ + { + word32 i; + byte useFake; + int realLen = keySz; + byte realPad[WC_SHA256_DIGEST_SIZE]; + + XMEMSET(realPad, 0, sizeof(realPad)); + /* Constant-time copy: avoid data-dependent branches + * that could leak whether RSA padding was valid. + * When outKey is NULL (inline RSA failure), use + * encryptedKey as a safe readable source; the mask + * will zero out all bytes anyway. Both encryptedKey + * and outKey (when non-NULL) are at least + * sizeof(realPad) bytes for any RSA key size. + * + * Use constant-time pointer selection to avoid + * branching on outKey nullity, which would leak + * whether RSA PKCS#1 v1.5 padding was valid. */ + { + byte haveSrc = ctMaskGTE(realLen, 1); + const byte* srcTbl[2]; + const byte* src; + word32 j = 0; + word32 safeJ = 0; + + /* Select source without integer pointer synthesis. + * Some safety-oriented compilers (e.g. Fil-C) treat + * int-to-pointer reconstruction as a null-object + * pointer on dereference. */ + srcTbl[0] = encryptedKey; + srcTbl[1] = outKey; + src = srcTbl[haveSrc & 1]; + + /* safeJ is clamped to max(0, realLen-1): it + * only advances while the next index would + * still be inside realLen, so src[safeJ] is + * always in bounds. Bytes at j >= realLen are + * masked to zero by inBounds anyway. */ + for (j = 0; j < (word32)sizeof(realPad); j++) { + byte inBounds = ctMaskLT((int)j, realLen); + byte advance = ctMaskLT((int)(safeJ + 1), + realLen); + realPad[j] = src[safeJ] & haveSrc & inBounds; + safeJ += (word32)(advance & 1); + } + } + useFake = ctMaskLT(realLen, 1); /* 0xFF if realLen<=0 */ + + for (i = 0; i < (word32)sizeof(fakeKey); i++) { + decryptedKey[i] = ctMaskSel(useFake, fakeKey[i], + realPad[i]); + } + /* Report the real key size on success; on RSA + * failure (realLen <= 0) report sizeof(fakeKey). + * Constant-time select avoids branching on RSA + * padding validity. */ + *decryptedKeySz = (word32)ctMaskSelInt(useFake, + (int)sizeof(fakeKey), realLen); + ForceZero(realPad, sizeof(realPad)); + } + ForceZero(fakeKey, sizeof(fakeKey)); + /* In the non-OAEP path RSA is decrypted in-place via + * wc_RsaPrivateDecryptInline, so encryptedKey holds the + * plaintext CEK after the unwrap. Zero it before free. */ + ForceZero(encryptedKey, (word32)encryptedKeySz); + } + #else /* NO_HMAC || NO_SHA256: mitigation unavailable */ + #if !defined(WOLFSSL_NO_KTRI_ORACLE_WARNING) + #warning "PKCS7 KTRI Bleichenbacher mitigation requires HMAC " \ + "and SHA256; build without them leaves the RSA unwrap " \ + "error path observable to callers. " \ + "Define WOLFSSL_NO_KTRI_ORACLE_WARNING to silence." + #endif + if (keySz <= 0 || outKey == NULL) { ForceZero(encryptedKey, (word32)encryptedKeySz); XFREE(encryptedKey, pkcs7->heap, DYNAMIC_TYPE_WOLF_BIGINT); WC_FREE_VAR_EX(privKey, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); - #ifndef WC_NO_RSA_OAEP + #ifndef WC_NO_RSA_OAEP if (encOID == RSAESOAEPk) { if (outKey) { ForceZero(outKey, outKeySz); XFREE(outKey, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); } } - #endif + #endif return keySz; - } else { + } + else { *decryptedKeySz = (word32)keySz; XMEMCPY(decryptedKey, outKey, (word32)keySz); ForceZero(encryptedKey, (word32)encryptedKeySz); } + #endif /* !NO_HMAC && !NO_SHA256 */ XFREE(encryptedKey, pkcs7->heap, DYNAMIC_TYPE_WOLF_BIGINT); WC_FREE_VAR_EX(privKey, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); @@ -12888,6 +13092,11 @@ int wc_PKCS7_DecodeEnvelopedData(wc_PKCS7* pkcs7, byte* in, DYNAMIC_TYPE_PKCS7); if (decryptedKey == NULL) return MEMORY_E; + XMEMSET(decryptedKey, 0, MAX_ENCRYPTED_KEY_SZ); + #ifdef WOLFSSL_CHECK_MEM_ZERO + wc_MemZero_Add("wc_PKCS7 decryptedKey", decryptedKey, + MAX_ENCRYPTED_KEY_SZ); + #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_ENV_2); tmpIdx = idx; recipientSetSz = (word32)ret; @@ -13130,10 +13339,18 @@ int wc_PKCS7_DecodeEnvelopedData(wc_PKCS7* pkcs7, byte* in, wc_PKCS7_StreamStoreVar(pkcs7, encOID, expBlockSz, explicitOctet); if (explicitOctet) { - /* initialize decryption state in preparation */ + /* initialize decryption state in preparation. Use + * contentSz (blockKeySz from the content algorithm) as + * the AES key size rather than aadSz (the unwrapped CEK + * length): the two are equal for well-formed messages, + * but using blockKeySz avoids BAD_FUNC_ARG on crafted + * messages where the CEK length does not match the + * content cipher, which would otherwise be a + * distinguishable error. */ if (pkcs7->decryptionCb == NULL) { ret = wc_PKCS7_DecryptContentInit(pkcs7, encOID, - pkcs7->stream->aad, pkcs7->stream->aadSz, + pkcs7->stream->aad, + (word32)pkcs7->stream->contentSz, pkcs7->stream->tmpIv, expBlockSz, pkcs7->devId, pkcs7->heap); if (ret != 0) @@ -13409,8 +13626,13 @@ int wc_PKCS7_DecodeEnvelopedData(wc_PKCS7* pkcs7, byte* in, ret = (int)pkcs7->totalEncryptedContentSz - padLen; #ifndef NO_PKCS7_STREAM - pkcs7->stream->aad = NULL; - pkcs7->stream->aadSz = 0; + /* decryptedKey (just freed) is the same buffer stream->aad + * aliases. Null the stream handle so ResetStream doesn't + * double-free it. */ + if (pkcs7->stream != NULL) { + pkcs7->stream->aad = NULL; + pkcs7->stream->aadSz = 0; + } wc_PKCS7_ResetStream(pkcs7); #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_START); @@ -13423,6 +13645,16 @@ int wc_PKCS7_DecodeEnvelopedData(wc_PKCS7* pkcs7, byte* in, #ifndef NO_PKCS7_STREAM if (ret < 0 && ret != WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)) { + /* stream->aad aliases the MAX_ENCRYPTED_KEY_SZ decryptedKey + * buffer in this flow. ResetStream only zeros aadSz bytes, so + * explicitly zero and release the full buffer here to satisfy + * WOLFSSL_CHECK_MEM_ZERO and avoid leaking key material. */ + if (pkcs7->stream != NULL && pkcs7->stream->aad != NULL) { + ForceZero(pkcs7->stream->aad, MAX_ENCRYPTED_KEY_SZ); + XFREE(pkcs7->stream->aad, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + pkcs7->stream->aad = NULL; + pkcs7->stream->aadSz = 0; + } wc_PKCS7_ResetStream(pkcs7); wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_START); if (pkcs7->cachedEncryptedContent != NULL) { @@ -14196,6 +14428,10 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in, } else { XMEMSET(decryptedKey, 0, MAX_ENCRYPTED_KEY_SZ); + #ifdef WOLFSSL_CHECK_MEM_ZERO + wc_MemZero_Add("wc_PKCS7 decryptedKey", decryptedKey, + MAX_ENCRYPTED_KEY_SZ); + #endif } #ifndef NO_PKCS7_STREAM pkcs7->stream->key = decryptedKey; From b7f6e77a95bfeda306b22dd34394e49c0a6c8a8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 21 Apr 2026 13:31:38 +0200 Subject: [PATCH 09/11] Reject PKCS#7 SignedData signer-identity forgery --- src/ssl_p7p12.c | 28 ++- tests/api/test_ossl_p7p12.c | 454 ++++++++++++++++++++++++++++++++++++ tests/api/test_ossl_p7p12.h | 4 + wolfcrypt/src/pkcs7.c | 159 +++++++++++++ 4 files changed, 640 insertions(+), 5 deletions(-) diff --git a/src/ssl_p7p12.c b/src/ssl_p7p12.c index ee48d700aa..a07f018b2a 100644 --- a/src/ssl_p7p12.c +++ b/src/ssl_p7p12.c @@ -269,24 +269,42 @@ WOLFSSL_STACK* wolfSSL_PKCS7_get0_signers(PKCS7* pkcs7, WOLFSSL_STACK* certs, WOLFSSL_X509* x509 = NULL; WOLFSSL_STACK* signers = NULL; WOLFSSL_PKCS7* p7 = (WOLFSSL_PKCS7*)pkcs7; + byte* signerCert; + word32 signerCertSz; if (p7 == NULL) return NULL; - /* Only PKCS#7 messages with a single cert that is the verifying certificate - * is supported. - */ if (flags & PKCS7_NOINTERN) { WOLFSSL_MSG("PKCS7_NOINTERN flag not supported"); return NULL; } + /* Prefer the certificate that actually verified the signature. Falling + * back to singleCert (cert[0]) would let an attacker that bundles a + * trusted cert ahead of their own attacker cert have the trusted cert + * reported as the signer even though it did not produce the signature. + * + * Copy the chosen pointer into a local before passing its address to + * wolfSSL_d2i_X509; d2i_X509 advances *in by the DER length, and if + * we handed it the address of the struct field directly it would + * permanently corrupt the field, producing a heap-OOB read on the + * next use (pointer advanced, singleCertSz unchanged). */ + if (p7->pkcs7.verifyCert != NULL && p7->pkcs7.verifyCertSz > 0) { + signerCert = p7->pkcs7.verifyCert; + signerCertSz = p7->pkcs7.verifyCertSz; + } + else { + signerCert = p7->pkcs7.singleCert; + signerCertSz = p7->pkcs7.singleCertSz; + } + signers = wolfSSL_sk_X509_new_null(); if (signers == NULL) return NULL; - if (wolfSSL_d2i_X509(&x509, (const byte**)&p7->pkcs7.singleCert, - p7->pkcs7.singleCertSz) == NULL) { + if (wolfSSL_d2i_X509(&x509, (const byte**)&signerCert, + signerCertSz) == NULL) { wolfSSL_sk_X509_pop_free(signers, NULL); return NULL; } diff --git a/tests/api/test_ossl_p7p12.c b/tests/api/test_ossl_p7p12.c index c92b80ee5f..03590a3ad7 100644 --- a/tests/api/test_ossl_p7p12.c +++ b/tests/api/test_ossl_p7p12.c @@ -446,6 +446,460 @@ int test_wolfSSL_PKCS7_sign(void) return EXPECT_RESULT(); } +/* Regression test for CMS SignedData signer-identity forgery. + * + * The embedded DER is a CMS SignedData message crafted so that the + * certificates SET contains two certificates: + * cert[0] = certs/ca-cert.pem (trusted wolfSSL CA; attacker does NOT hold + * its private key) + * cert[1] = a self-signed "attacker" P-256 certificate (attacker holds + * the private key) + * The signerInfo sid names the attacker certificate, and the signature + * was produced with the attacker's key over "Hello World". + * + * The bug that was present: wolfSSL_PKCS7_verify() iterated all bundled + * certificates trying each public key against the signature. When the + * attacker's key verified, it still reported cert[0] (the trusted CA cert, + * via singleCert) as the signer, and chain validation therefore succeeded + * on an unrelated trusted cert - a full signer-identity forgery. + * + * The expected, correct behavior: the CMS message is rejected because the + * signer certificate named by the sid (the attacker cert) does not chain + * to any certificate in the trust store. */ +int test_wolfSSL_PKCS7_verify_signer_forgery(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_ALL) && defined(HAVE_PKCS7) && !defined(NO_BIO) && \ + !defined(NO_FILESYSTEM) && !defined(NO_RSA) && defined(HAVE_ECC) + static const byte forgedSignedData[] = { + 0x30, 0x82, 0x07, 0x9c, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, + 0x01, 0x07, 0x02, 0xa0, 0x82, 0x07, 0x8d, 0x30, 0x82, 0x07, 0x89, 0x02, + 0x01, 0x01, 0x31, 0x0d, 0x30, 0x0b, 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, + 0x65, 0x03, 0x04, 0x02, 0x01, 0x30, 0x1b, 0x06, 0x09, 0x2a, 0x86, 0x48, + 0x86, 0xf7, 0x0d, 0x01, 0x07, 0x01, 0xa0, 0x0e, 0x04, 0x0c, 0x48, 0x65, + 0x6c, 0x6c, 0x6f, 0x20, 0x57, 0x6f, 0x72, 0x6c, 0x64, 0x0a, 0xa0, 0x82, + 0x06, 0xab, 0x30, 0x82, 0x04, 0xff, 0x30, 0x82, 0x03, 0xe7, 0xa0, 0x03, + 0x02, 0x01, 0x02, 0x02, 0x14, 0x3f, 0x29, 0x11, 0x20, 0x57, 0x71, 0xe7, + 0x8e, 0xf9, 0x18, 0x0d, 0xca, 0x70, 0x4d, 0x5b, 0x15, 0x2a, 0x43, 0xd6, + 0x24, 0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, + 0x01, 0x0b, 0x05, 0x00, 0x30, 0x81, 0x94, 0x31, 0x0b, 0x30, 0x09, 0x06, + 0x03, 0x55, 0x04, 0x06, 0x13, 0x02, 0x55, 0x53, 0x31, 0x10, 0x30, 0x0e, + 0x06, 0x03, 0x55, 0x04, 0x08, 0x0c, 0x07, 0x4d, 0x6f, 0x6e, 0x74, 0x61, + 0x6e, 0x61, 0x31, 0x10, 0x30, 0x0e, 0x06, 0x03, 0x55, 0x04, 0x07, 0x0c, + 0x07, 0x42, 0x6f, 0x7a, 0x65, 0x6d, 0x61, 0x6e, 0x31, 0x11, 0x30, 0x0f, + 0x06, 0x03, 0x55, 0x04, 0x0a, 0x0c, 0x08, 0x53, 0x61, 0x77, 0x74, 0x6f, + 0x6f, 0x74, 0x68, 0x31, 0x13, 0x30, 0x11, 0x06, 0x03, 0x55, 0x04, 0x0b, + 0x0c, 0x0a, 0x43, 0x6f, 0x6e, 0x73, 0x75, 0x6c, 0x74, 0x69, 0x6e, 0x67, + 0x31, 0x18, 0x30, 0x16, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c, 0x0f, 0x77, + 0x77, 0x77, 0x2e, 0x77, 0x6f, 0x6c, 0x66, 0x73, 0x73, 0x6c, 0x2e, 0x63, + 0x6f, 0x6d, 0x31, 0x1f, 0x30, 0x1d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, + 0xf7, 0x0d, 0x01, 0x09, 0x01, 0x16, 0x10, 0x69, 0x6e, 0x66, 0x6f, 0x40, + 0x77, 0x6f, 0x6c, 0x66, 0x73, 0x73, 0x6c, 0x2e, 0x63, 0x6f, 0x6d, 0x30, + 0x1e, 0x17, 0x0d, 0x32, 0x35, 0x31, 0x31, 0x31, 0x33, 0x32, 0x30, 0x34, + 0x31, 0x31, 0x31, 0x5a, 0x17, 0x0d, 0x32, 0x38, 0x30, 0x38, 0x30, 0x39, + 0x32, 0x30, 0x34, 0x31, 0x31, 0x31, 0x5a, 0x30, 0x81, 0x94, 0x31, 0x0b, + 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x06, 0x13, 0x02, 0x55, 0x53, 0x31, + 0x10, 0x30, 0x0e, 0x06, 0x03, 0x55, 0x04, 0x08, 0x0c, 0x07, 0x4d, 0x6f, + 0x6e, 0x74, 0x61, 0x6e, 0x61, 0x31, 0x10, 0x30, 0x0e, 0x06, 0x03, 0x55, + 0x04, 0x07, 0x0c, 0x07, 0x42, 0x6f, 0x7a, 0x65, 0x6d, 0x61, 0x6e, 0x31, + 0x11, 0x30, 0x0f, 0x06, 0x03, 0x55, 0x04, 0x0a, 0x0c, 0x08, 0x53, 0x61, + 0x77, 0x74, 0x6f, 0x6f, 0x74, 0x68, 0x31, 0x13, 0x30, 0x11, 0x06, 0x03, + 0x55, 0x04, 0x0b, 0x0c, 0x0a, 0x43, 0x6f, 0x6e, 0x73, 0x75, 0x6c, 0x74, + 0x69, 0x6e, 0x67, 0x31, 0x18, 0x30, 0x16, 0x06, 0x03, 0x55, 0x04, 0x03, + 0x0c, 0x0f, 0x77, 0x77, 0x77, 0x2e, 0x77, 0x6f, 0x6c, 0x66, 0x73, 0x73, + 0x6c, 0x2e, 0x63, 0x6f, 0x6d, 0x31, 0x1f, 0x30, 0x1d, 0x06, 0x09, 0x2a, + 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x09, 0x01, 0x16, 0x10, 0x69, 0x6e, + 0x66, 0x6f, 0x40, 0x77, 0x6f, 0x6c, 0x66, 0x73, 0x73, 0x6c, 0x2e, 0x63, + 0x6f, 0x6d, 0x30, 0x82, 0x01, 0x22, 0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, + 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x01, 0x01, 0x05, 0x00, 0x03, 0x82, 0x01, + 0x0f, 0x00, 0x30, 0x82, 0x01, 0x0a, 0x02, 0x82, 0x01, 0x01, 0x00, 0xbf, + 0x0c, 0xca, 0x2d, 0x14, 0xb2, 0x1e, 0x84, 0x42, 0x5b, 0xcd, 0x38, 0x1f, + 0x4a, 0xf2, 0x4d, 0x75, 0x10, 0xf1, 0xb6, 0x35, 0x9f, 0xdf, 0xca, 0x7d, + 0x03, 0x98, 0xd3, 0xac, 0xde, 0x03, 0x66, 0xee, 0x2a, 0xf1, 0xd8, 0xb0, + 0x7d, 0x6e, 0x07, 0x54, 0x0b, 0x10, 0x98, 0x21, 0x4d, 0x80, 0xcb, 0x12, + 0x20, 0xe7, 0xcc, 0x4f, 0xde, 0x45, 0x7d, 0xc9, 0x72, 0x77, 0x32, 0xea, + 0xca, 0x90, 0xbb, 0x69, 0x52, 0x10, 0x03, 0x2f, 0xa8, 0xf3, 0x95, 0xc5, + 0xf1, 0x8b, 0x62, 0x56, 0x1b, 0xef, 0x67, 0x6f, 0xa4, 0x10, 0x41, 0x95, + 0xad, 0x0a, 0x9b, 0xe3, 0xa5, 0xc0, 0xb0, 0xd2, 0x70, 0x76, 0x50, 0x30, + 0x5b, 0xa8, 0xe8, 0x08, 0x2c, 0x7c, 0xed, 0xa7, 0xa2, 0x7a, 0x8d, 0x38, + 0x29, 0x1c, 0xac, 0xc7, 0xed, 0xf2, 0x7c, 0x95, 0xb0, 0x95, 0x82, 0x7d, + 0x49, 0x5c, 0x38, 0xcd, 0x77, 0x25, 0xef, 0xbd, 0x80, 0x75, 0x53, 0x94, + 0x3c, 0x3d, 0xca, 0x63, 0x5b, 0x9f, 0x15, 0xb5, 0xd3, 0x1d, 0x13, 0x2f, + 0x19, 0xd1, 0x3c, 0xdb, 0x76, 0x3a, 0xcc, 0xb8, 0x7d, 0xc9, 0xe5, 0xc2, + 0xd7, 0xda, 0x40, 0x6f, 0xd8, 0x21, 0xdc, 0x73, 0x1b, 0x42, 0x2d, 0x53, + 0x9c, 0xfe, 0x1a, 0xfc, 0x7d, 0xab, 0x7a, 0x36, 0x3f, 0x98, 0xde, 0x84, + 0x7c, 0x05, 0x67, 0xce, 0x6a, 0x14, 0x38, 0x87, 0xa9, 0xf1, 0x8c, 0xb5, + 0x68, 0xcb, 0x68, 0x7f, 0x71, 0x20, 0x2b, 0xf5, 0xa0, 0x63, 0xf5, 0x56, + 0x2f, 0xa3, 0x26, 0xd2, 0xb7, 0x6f, 0xb1, 0x5a, 0x17, 0xd7, 0x38, 0x99, + 0x08, 0xfe, 0x93, 0x58, 0x6f, 0xfe, 0xc3, 0x13, 0x49, 0x08, 0x16, 0x0b, + 0xa7, 0x4d, 0x67, 0x00, 0x52, 0x31, 0x67, 0x23, 0x4e, 0x98, 0xed, 0x51, + 0x45, 0x1d, 0xb9, 0x04, 0xd9, 0x0b, 0xec, 0xd8, 0x28, 0xb3, 0x4b, 0xbd, + 0xed, 0x36, 0x79, 0x02, 0x03, 0x01, 0x00, 0x01, 0xa3, 0x82, 0x01, 0x45, + 0x30, 0x82, 0x01, 0x41, 0x30, 0x1d, 0x06, 0x03, 0x55, 0x1d, 0x0e, 0x04, + 0x16, 0x04, 0x14, 0x27, 0x8e, 0x67, 0x11, 0x74, 0xc3, 0x26, 0x1d, 0x3f, + 0xed, 0x33, 0x63, 0xb3, 0xa4, 0xd8, 0x1d, 0x30, 0xe5, 0xe8, 0xd5, 0x30, + 0x81, 0xd4, 0x06, 0x03, 0x55, 0x1d, 0x23, 0x04, 0x81, 0xcc, 0x30, 0x81, + 0xc9, 0x80, 0x14, 0x27, 0x8e, 0x67, 0x11, 0x74, 0xc3, 0x26, 0x1d, 0x3f, + 0xed, 0x33, 0x63, 0xb3, 0xa4, 0xd8, 0x1d, 0x30, 0xe5, 0xe8, 0xd5, 0xa1, + 0x81, 0x9a, 0xa4, 0x81, 0x97, 0x30, 0x81, 0x94, 0x31, 0x0b, 0x30, 0x09, + 0x06, 0x03, 0x55, 0x04, 0x06, 0x13, 0x02, 0x55, 0x53, 0x31, 0x10, 0x30, + 0x0e, 0x06, 0x03, 0x55, 0x04, 0x08, 0x0c, 0x07, 0x4d, 0x6f, 0x6e, 0x74, + 0x61, 0x6e, 0x61, 0x31, 0x10, 0x30, 0x0e, 0x06, 0x03, 0x55, 0x04, 0x07, + 0x0c, 0x07, 0x42, 0x6f, 0x7a, 0x65, 0x6d, 0x61, 0x6e, 0x31, 0x11, 0x30, + 0x0f, 0x06, 0x03, 0x55, 0x04, 0x0a, 0x0c, 0x08, 0x53, 0x61, 0x77, 0x74, + 0x6f, 0x6f, 0x74, 0x68, 0x31, 0x13, 0x30, 0x11, 0x06, 0x03, 0x55, 0x04, + 0x0b, 0x0c, 0x0a, 0x43, 0x6f, 0x6e, 0x73, 0x75, 0x6c, 0x74, 0x69, 0x6e, + 0x67, 0x31, 0x18, 0x30, 0x16, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c, 0x0f, + 0x77, 0x77, 0x77, 0x2e, 0x77, 0x6f, 0x6c, 0x66, 0x73, 0x73, 0x6c, 0x2e, + 0x63, 0x6f, 0x6d, 0x31, 0x1f, 0x30, 0x1d, 0x06, 0x09, 0x2a, 0x86, 0x48, + 0x86, 0xf7, 0x0d, 0x01, 0x09, 0x01, 0x16, 0x10, 0x69, 0x6e, 0x66, 0x6f, + 0x40, 0x77, 0x6f, 0x6c, 0x66, 0x73, 0x73, 0x6c, 0x2e, 0x63, 0x6f, 0x6d, + 0x82, 0x14, 0x3f, 0x29, 0x11, 0x20, 0x57, 0x71, 0xe7, 0x8e, 0xf9, 0x18, + 0x0d, 0xca, 0x70, 0x4d, 0x5b, 0x15, 0x2a, 0x43, 0xd6, 0x24, 0x30, 0x0c, + 0x06, 0x03, 0x55, 0x1d, 0x13, 0x04, 0x05, 0x30, 0x03, 0x01, 0x01, 0xff, + 0x30, 0x1c, 0x06, 0x03, 0x55, 0x1d, 0x11, 0x04, 0x15, 0x30, 0x13, 0x82, + 0x0b, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x2e, 0x63, 0x6f, 0x6d, + 0x87, 0x04, 0x7f, 0x00, 0x00, 0x01, 0x30, 0x1d, 0x06, 0x03, 0x55, 0x1d, + 0x25, 0x04, 0x16, 0x30, 0x14, 0x06, 0x08, 0x2b, 0x06, 0x01, 0x05, 0x05, + 0x07, 0x03, 0x01, 0x06, 0x08, 0x2b, 0x06, 0x01, 0x05, 0x05, 0x07, 0x03, + 0x02, 0x30, 0x0d, 0x06, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, + 0x01, 0x0b, 0x05, 0x00, 0x03, 0x82, 0x01, 0x01, 0x00, 0x0f, 0xae, 0x89, + 0xd5, 0x68, 0xe4, 0x41, 0xf8, 0x9b, 0xe0, 0xc5, 0x61, 0x06, 0x57, 0xff, + 0xa0, 0x92, 0x0f, 0xb2, 0xed, 0xd3, 0x99, 0x5b, 0x99, 0x5e, 0x32, 0x7e, + 0x97, 0xc7, 0xaf, 0x6c, 0xfe, 0x8c, 0xa6, 0xae, 0x32, 0xa1, 0x0d, 0xca, + 0xcd, 0xfc, 0x18, 0xe5, 0xd1, 0xf8, 0x20, 0x5b, 0x5a, 0x38, 0x81, 0x46, + 0x5b, 0x48, 0x87, 0xa5, 0x3f, 0x3b, 0x7b, 0xc7, 0xea, 0xf5, 0x35, 0x29, + 0x31, 0x15, 0x39, 0x38, 0x5d, 0x48, 0xe6, 0x01, 0x81, 0x5c, 0x5e, 0x7c, + 0x10, 0xf5, 0x16, 0xe3, 0x59, 0xaf, 0x44, 0xc8, 0xb5, 0x8d, 0xc1, 0x32, + 0x23, 0xb3, 0xb8, 0x12, 0x6e, 0x5c, 0x8d, 0xe6, 0xc2, 0xd2, 0x41, 0x03, + 0xeb, 0x17, 0x42, 0xe2, 0x7f, 0xbc, 0x00, 0x5d, 0xa5, 0x31, 0xef, 0xc6, + 0x48, 0xee, 0xdb, 0xcc, 0xe0, 0xf1, 0x56, 0xf5, 0xd4, 0xca, 0x45, 0xa1, + 0x59, 0xb5, 0xe4, 0xd7, 0x60, 0x9c, 0x57, 0xe0, 0xa7, 0x5a, 0xf2, 0x35, + 0x1e, 0xa0, 0x22, 0xdb, 0x5e, 0x1c, 0x0c, 0x61, 0xbd, 0xa1, 0xc5, 0x7b, + 0x9f, 0x69, 0xf2, 0xd5, 0x95, 0xe2, 0xbc, 0x52, 0xb9, 0x1d, 0x9c, 0x2c, + 0xda, 0xb6, 0x73, 0x75, 0x4a, 0x84, 0xe5, 0x94, 0xb8, 0x19, 0x4d, 0xdd, + 0x70, 0xbd, 0x7f, 0x4c, 0xb9, 0x17, 0x6a, 0x58, 0x16, 0x89, 0x22, 0x44, + 0x37, 0x57, 0x55, 0x26, 0x42, 0xe3, 0xb7, 0xe5, 0xc7, 0x2b, 0x40, 0x0c, + 0xe9, 0xe4, 0x7f, 0x52, 0x75, 0xdf, 0x06, 0xc9, 0xfb, 0x01, 0x44, 0x34, + 0xac, 0x20, 0x3c, 0xb4, 0xbe, 0x2b, 0x3e, 0xef, 0x85, 0x38, 0x96, 0x5b, + 0x9b, 0x1e, 0x25, 0x86, 0x18, 0x4c, 0xa4, 0x06, 0x70, 0x06, 0x6a, 0xc8, + 0x4b, 0x6f, 0x5f, 0xc4, 0x05, 0x1f, 0x03, 0x62, 0x30, 0x11, 0x61, 0xbc, + 0xc1, 0x40, 0x31, 0x66, 0xdc, 0x64, 0xf0, 0x4f, 0x6b, 0xb9, 0xec, 0xc8, + 0x29, 0x30, 0x82, 0x01, 0xa4, 0x30, 0x82, 0x01, 0x49, 0xa0, 0x03, 0x02, + 0x01, 0x02, 0x02, 0x14, 0x62, 0x4d, 0x11, 0x9c, 0xcf, 0x5d, 0xe5, 0x71, + 0xa2, 0x82, 0xd9, 0x8f, 0xe0, 0x04, 0xb8, 0x5f, 0x0e, 0x4d, 0x07, 0xad, + 0x30, 0x0a, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x03, 0x02, + 0x30, 0x27, 0x31, 0x11, 0x30, 0x0f, 0x06, 0x03, 0x55, 0x04, 0x03, 0x0c, + 0x08, 0x61, 0x74, 0x74, 0x61, 0x63, 0x6b, 0x65, 0x72, 0x31, 0x12, 0x30, + 0x10, 0x06, 0x03, 0x55, 0x04, 0x0a, 0x0c, 0x09, 0x75, 0x6e, 0x74, 0x72, + 0x75, 0x73, 0x74, 0x65, 0x64, 0x30, 0x1e, 0x17, 0x0d, 0x32, 0x36, 0x30, + 0x34, 0x32, 0x31, 0x31, 0x31, 0x31, 0x36, 0x32, 0x38, 0x5a, 0x17, 0x0d, + 0x33, 0x36, 0x30, 0x34, 0x31, 0x38, 0x31, 0x31, 0x31, 0x36, 0x32, 0x38, + 0x5a, 0x30, 0x27, 0x31, 0x11, 0x30, 0x0f, 0x06, 0x03, 0x55, 0x04, 0x03, + 0x0c, 0x08, 0x61, 0x74, 0x74, 0x61, 0x63, 0x6b, 0x65, 0x72, 0x31, 0x12, + 0x30, 0x10, 0x06, 0x03, 0x55, 0x04, 0x0a, 0x0c, 0x09, 0x75, 0x6e, 0x74, + 0x72, 0x75, 0x73, 0x74, 0x65, 0x64, 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, 0xae, 0xdb, 0xf7, + 0x3b, 0x7e, 0x82, 0x88, 0xfc, 0x1a, 0xfb, 0x86, 0x56, 0x83, 0x03, 0xdd, + 0x05, 0x14, 0x79, 0x51, 0x0f, 0x3c, 0x86, 0x85, 0x2d, 0xeb, 0x18, 0x17, + 0x20, 0x3b, 0x37, 0x6f, 0x7f, 0x78, 0x19, 0x3b, 0xf6, 0x71, 0xad, 0xc9, + 0x65, 0x81, 0x7e, 0xe0, 0xa9, 0x29, 0xdd, 0xfd, 0xf0, 0xff, 0x04, 0x7d, + 0x5a, 0x59, 0xd6, 0x6c, 0xe2, 0xde, 0xc5, 0xd5, 0xb6, 0x1f, 0x69, 0xd9, + 0x33, 0xa3, 0x53, 0x30, 0x51, 0x30, 0x1d, 0x06, 0x03, 0x55, 0x1d, 0x0e, + 0x04, 0x16, 0x04, 0x14, 0xf7, 0xab, 0x3f, 0x49, 0xcf, 0x7d, 0x48, 0x9c, + 0x04, 0x49, 0x1a, 0xac, 0x8f, 0x26, 0x16, 0x09, 0xa8, 0x2a, 0x74, 0xf5, + 0x30, 0x1f, 0x06, 0x03, 0x55, 0x1d, 0x23, 0x04, 0x18, 0x30, 0x16, 0x80, + 0x14, 0xf7, 0xab, 0x3f, 0x49, 0xcf, 0x7d, 0x48, 0x9c, 0x04, 0x49, 0x1a, + 0xac, 0x8f, 0x26, 0x16, 0x09, 0xa8, 0x2a, 0x74, 0xf5, 0x30, 0x0f, 0x06, + 0x03, 0x55, 0x1d, 0x13, 0x01, 0x01, 0xff, 0x04, 0x05, 0x30, 0x03, 0x01, + 0x01, 0xff, 0x30, 0x0a, 0x06, 0x08, 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, + 0x03, 0x02, 0x03, 0x49, 0x00, 0x30, 0x46, 0x02, 0x21, 0x00, 0x8d, 0xbf, + 0x36, 0xe5, 0x51, 0x9a, 0xde, 0xf4, 0x7f, 0xbf, 0xbd, 0x7f, 0x71, 0x66, + 0xc1, 0x67, 0xfa, 0x71, 0x0d, 0x79, 0xc6, 0x60, 0x3a, 0x6c, 0xeb, 0x43, + 0xc3, 0xf2, 0x5e, 0xe8, 0x74, 0xb6, 0x02, 0x21, 0x00, 0xfa, 0xdb, 0x40, + 0x47, 0x72, 0xf0, 0x15, 0x52, 0xc1, 0x78, 0x11, 0x6b, 0x76, 0xc5, 0x1f, + 0xcf, 0xb6, 0x09, 0x6d, 0x8f, 0xcb, 0x92, 0x2f, 0x1b, 0x3c, 0xc3, 0x28, + 0x48, 0x61, 0x0f, 0x60, 0x71, 0x31, 0x81, 0xa8, 0x30, 0x81, 0xa5, 0x02, + 0x01, 0x01, 0x30, 0x3f, 0x30, 0x27, 0x31, 0x11, 0x30, 0x0f, 0x06, 0x03, + 0x55, 0x04, 0x03, 0x0c, 0x08, 0x61, 0x74, 0x74, 0x61, 0x63, 0x6b, 0x65, + 0x72, 0x31, 0x12, 0x30, 0x10, 0x06, 0x03, 0x55, 0x04, 0x0a, 0x0c, 0x09, + 0x75, 0x6e, 0x74, 0x72, 0x75, 0x73, 0x74, 0x65, 0x64, 0x02, 0x14, 0x62, + 0x4d, 0x11, 0x9c, 0xcf, 0x5d, 0xe5, 0x71, 0xa2, 0x82, 0xd9, 0x8f, 0xe0, + 0x04, 0xb8, 0x5f, 0x0e, 0x4d, 0x07, 0xad, 0x30, 0x0b, 0x06, 0x09, 0x60, + 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, 0x30, 0x0a, 0x06, 0x08, + 0x2a, 0x86, 0x48, 0xce, 0x3d, 0x04, 0x03, 0x02, 0x04, 0x46, 0x30, 0x44, + 0x02, 0x20, 0x22, 0x4a, 0x99, 0xb1, 0xbc, 0xa9, 0xee, 0x24, 0x60, 0x81, + 0xb9, 0x64, 0xba, 0x86, 0x00, 0xae, 0xb5, 0xd7, 0xb8, 0x72, 0xb9, 0x8c, + 0xb3, 0xe7, 0x78, 0x29, 0xdb, 0xa8, 0x27, 0xf7, 0x30, 0xf0, 0x02, 0x20, + 0x19, 0x2d, 0xd3, 0x17, 0x9a, 0xc1, 0xf9, 0xd2, 0x63, 0x92, 0x8e, 0x78, + 0xcc, 0xa4, 0x0b, 0x91, 0x12, 0xa5, 0xb2, 0xbc, 0x35, 0x87, 0x8e, 0x33, + 0xa7, 0xe0, 0x5e, 0xab, 0x95, 0xb2, 0x2a, 0xf4 + }; + PKCS7* p7 = NULL; + X509_STORE* store = NULL; + X509* caCert = NULL; + WOLFSSL_BIO* caBio = NULL; + const byte* p = forgedSignedData; + const char* ca = "./certs/ca-cert.pem"; + + /* Load the same CA into the trust store that the attacker bundled at + * cert[0] in the forged message. */ + ExpectNotNull(caBio = BIO_new_file(ca, "r")); + ExpectNotNull(caCert = PEM_read_bio_X509(caBio, NULL, 0, NULL)); + ExpectNotNull(store = X509_STORE_new()); + ExpectIntEQ(X509_STORE_add_cert(store, caCert), 1); + + /* Parse the forged message. d2i_PKCS7 internally runs + * wc_PKCS7_VerifySignedData, which must NOT accept the attacker's + * signature under any bundled cert other than the one named by the + * signerInfo sid. Since the sid names the attacker cert (which does + * not chain to the trusted CA), the parse may succeed but verification + * against the trust store must fail. */ + ExpectNotNull(p7 = d2i_PKCS7(NULL, &p, (int)sizeof(forgedSignedData))); + + /* PKCS7_verify() MUST fail: the only certificate in the trust store + * is the wolfSSL CA - it is bundled at cert[0] but did NOT sign this + * message. The actual signer (the attacker's self-signed cert at + * cert[1]) cannot chain to any trust anchor. */ + ExpectIntEQ(PKCS7_verify(p7, NULL, store, NULL, NULL, 0), + WC_NO_ERR_TRACE(WOLFSSL_FAILURE)); + + PKCS7_free(p7); + X509_STORE_free(store); + X509_free(caCert); + BIO_free(caBio); +#endif + return EXPECT_RESULT(); +} + +/* Exercise the SignerInfo-sid binding enforcement end-to-end. + * + * For both supported sid encodings (v1 = IssuerAndSerialNumber, v3 = + * SubjectKeyIdentifier), this builds a valid CMS SignedData message with + * two certificates in the bundle: + * cert[0] = ca-cert (extra, non-signing) + * cert[1] = server-cert (actual signer) + * and checks that: + * - parsing + signature verification succeeds, + * - chain validation against a trust store containing ca-cert succeeds, + * - PKCS7_get0_signers() returns the *signer* (server-cert), not the + * extra cert at cert[0] - which would be the pre-fix behavior and the + * core of the signer-identity forgery bug. */ +int test_wolfSSL_PKCS7_verify_sid_binding(void) +{ + EXPECT_DECLS; +#if defined(OPENSSL_ALL) && defined(HAVE_PKCS7) && !defined(NO_BIO) && \ + !defined(NO_FILESYSTEM) && !defined(NO_RSA) + const char* signerCertFile = "./certs/server-cert.pem"; + const char* signerKeyFile = "./certs/server-key.pem"; + const char* caFile = "./certs/ca-cert.pem"; + const byte content[] = "sid-binding test content"; + /* Build both variants: default v1 (IssuerAndSerialNumber) and v3 + * (SubjectKeyIdentifier). */ + const int sidTypes[2] = { CMS_ISSUER_AND_SERIAL_NUMBER, CMS_SKID }; + int variant; + + BIO* signerCertBio = NULL; + BIO* caBio = NULL; + X509* signerCertX509 = NULL; + X509* caX509 = NULL; + byte* signerCertDer = NULL; + byte* caDer = NULL; + byte* signerKey = NULL; + int signerCertDerSz = 0; + int caDerSz = 0; + size_t signerKeySz = 0; + XFILE keyFile = XBADFILE; + WC_RNG rng; + int rngInited = 0; + + /* ---- Load signer cert + key and the CA cert. ---- */ + ExpectNotNull(signerCertBio = BIO_new_file(signerCertFile, "r")); + ExpectNotNull(signerCertX509 = PEM_read_bio_X509(signerCertBio, NULL, 0, + NULL)); + ExpectIntGT(signerCertDerSz = i2d_X509(signerCertX509, &signerCertDer), 0); + + ExpectNotNull(caBio = BIO_new_file(caFile, "r")); + ExpectNotNull(caX509 = PEM_read_bio_X509(caBio, NULL, 0, NULL)); + ExpectIntGT(caDerSz = i2d_X509(caX509, &caDer), 0); + + /* Slurp the DER private key straight from a PEM->DER round-trip via + * wc_KeyPemToDer. The test only needs the bytes in a form + * wc_PKCS7_EncodeSignedData can consume. */ + { + long filePemLen = 0; + byte* keyPem = NULL; + int derLen = 0; + + ExpectTrue((keyFile = XFOPEN(signerKeyFile, "rb")) != XBADFILE); + if (keyFile != XBADFILE) { + (void)XFSEEK(keyFile, 0, XSEEK_END); + filePemLen = XFTELL(keyFile); + (void)XFSEEK(keyFile, 0, XSEEK_SET); + ExpectIntGT(filePemLen, 0); + keyPem = (byte*)XMALLOC((size_t)filePemLen, NULL, + DYNAMIC_TYPE_TMP_BUFFER); + ExpectNotNull(keyPem); + if (keyPem != NULL) { + ExpectIntEQ(XFREAD(keyPem, 1, (size_t)filePemLen, keyFile), + (size_t)filePemLen); + /* First call sizes the output buffer. */ + derLen = wc_KeyPemToDer(keyPem, (word32)filePemLen, NULL, 0, + NULL); + ExpectIntGT(derLen, 0); + if (derLen > 0) { + signerKey = (byte*)XMALLOC((size_t)derLen, NULL, + DYNAMIC_TYPE_TMP_BUFFER); + ExpectNotNull(signerKey); + if (signerKey != NULL) { + derLen = wc_KeyPemToDer(keyPem, (word32)filePemLen, + signerKey, (word32)derLen, + NULL); + ExpectIntGT(derLen, 0); + signerKeySz = (size_t)derLen; + } + } + } + XFREE(keyPem, NULL, DYNAMIC_TYPE_TMP_BUFFER); + XFCLOSE(keyFile); + keyFile = XBADFILE; + } + } + + ExpectIntEQ(wc_InitRng(&rng), 0); + if (EXPECT_SUCCESS()) + rngInited = 1; + + for (variant = 0; variant < 2; variant++) { + wc_PKCS7* p7Enc = NULL; + byte encoded[4096]; + int encodedSz = 0; + PKCS7* p7Ver = NULL; + X509_STORE* store = NULL; + const byte* encodedPtr = NULL; + STACK_OF(X509)* signers = NULL; + X509* reportedSigner = NULL; + byte* reportedSignerDer = NULL; + int reportedSignerDerSz = 0; + X509* caForStore = NULL; + BIO* caForStoreBio = NULL; + + /* ---- Encode: signer=server-cert, extra bundle cert=ca. ---- */ + ExpectNotNull(p7Enc = wc_PKCS7_New(HEAP_HINT, INVALID_DEVID)); + ExpectIntEQ(wc_PKCS7_Init(p7Enc, HEAP_HINT, INVALID_DEVID), 0); + ExpectIntEQ(wc_PKCS7_InitWithCert(p7Enc, signerCertDer, + (word32)signerCertDerSz), 0); + /* wc_PKCS7_AddCertificate prepends to the cert list - the encoded + * SET therefore ends up [ca, signer], putting the actual signer + * at index 1 and exercising the sid-selection path (cert[0] is + * NOT the signer and must be skipped). */ + ExpectIntEQ(wc_PKCS7_AddCertificate(p7Enc, caDer, (word32)caDerSz), 0); + + if (p7Enc != NULL) { + p7Enc->content = (byte*)content; + p7Enc->contentSz = (word32)sizeof(content); + p7Enc->encryptOID = RSAk; + p7Enc->hashOID = SHA256h; + p7Enc->privateKey = signerKey; + p7Enc->privateKeySz = (word32)signerKeySz; + p7Enc->rng = &rng; + } + + ExpectIntEQ(wc_PKCS7_SetSignerIdentifierType(p7Enc, sidTypes[variant]), + 0); + + ExpectIntGT((encodedSz = wc_PKCS7_EncodeSignedData(p7Enc, encoded, + sizeof(encoded))), + 0); + wc_PKCS7_Free(p7Enc); + p7Enc = NULL; + + /* ---- Parse + verify through the OpenSSL compat layer. ---- */ + encodedPtr = encoded; + ExpectNotNull(p7Ver = d2i_PKCS7(NULL, &encodedPtr, encodedSz)); + + /* Trust store holds only ca-cert. Reload it rather than reusing + * caX509, since X509_STORE_free takes ownership-like semantics. */ + ExpectNotNull(caForStoreBio = BIO_new_file(caFile, "r")); + ExpectNotNull(caForStore = PEM_read_bio_X509(caForStoreBio, NULL, 0, + NULL)); + ExpectNotNull(store = X509_STORE_new()); + ExpectIntEQ(X509_STORE_add_cert(store, caForStore), 1); + + ExpectIntEQ(PKCS7_verify(p7Ver, NULL, store, NULL, NULL, 0), 1); + + /* Snapshot the singleCert / verifyCert pointers and sizes after + * PKCS7_verify has finished re-parsing the message. A buggy + * implementation that passes &p7->pkcs7.singleCert (or verifyCert) + * directly to wolfSSL_d2i_X509 permanently advances the struct + * field, which corrupts it for any subsequent use - producing a + * heap-OOB read on the next call since the size isn't advanced. + * These pointers must stay exactly where they are across repeated + * get0_signers calls. */ + if (p7Ver != NULL) { + wc_PKCS7* wcP7 = &((WOLFSSL_PKCS7*)p7Ver)->pkcs7; + byte* singleBefore = wcP7->singleCert; + word32 singleSzBefore = wcP7->singleCertSz; + byte* verifyBefore = wcP7->verifyCert; + word32 verifySzBefore = wcP7->verifyCertSz; + int i; + + /* Call get0_signers repeatedly. Each invocation must return + * the correct cert and must not mutate singleCert/verifyCert. + * Three iterations so the "second call reads past the end" + * pattern (the exact OOB the reporter hit) is exercised. */ + for (i = 0; i < 3; i++) { + ExpectNotNull(signers = PKCS7_get0_signers(p7Ver, NULL, 0)); + ExpectIntEQ(sk_X509_num(signers), 1); + ExpectNotNull(reportedSigner = sk_X509_value(signers, 0)); + ExpectIntGT(reportedSignerDerSz = i2d_X509(reportedSigner, + &reportedSignerDer), 0); + /* DER-compare: reportedSigner must equal server-cert and + * must NOT equal ca-cert (the pre-fix signer-confusion + * outcome). */ + ExpectIntEQ(reportedSignerDerSz, signerCertDerSz); + if (reportedSignerDer != NULL && signerCertDer != NULL) { + ExpectIntEQ(XMEMCMP(reportedSignerDer, signerCertDer, + (size_t)signerCertDerSz), 0); + if (reportedSignerDerSz == caDerSz) { + ExpectIntNE(XMEMCMP(reportedSignerDer, caDer, + (size_t)caDerSz), 0); + } + } + XFREE(reportedSignerDer, NULL, DYNAMIC_TYPE_OPENSSL); + reportedSignerDer = NULL; + sk_X509_pop_free(signers, NULL); + signers = NULL; + + /* Struct fields must survive every call unchanged. */ + ExpectPtrEq(wcP7->singleCert, singleBefore); + ExpectIntEQ(wcP7->singleCertSz, singleSzBefore); + ExpectPtrEq(wcP7->verifyCert, verifyBefore); + ExpectIntEQ(wcP7->verifyCertSz, verifySzBefore); + } + } + + PKCS7_free(p7Ver); + X509_STORE_free(store); + X509_free(caForStore); + BIO_free(caForStoreBio); + } + + if (rngInited) + wc_FreeRng(&rng); + + XFREE(signerKey, NULL, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(signerCertDer, NULL, DYNAMIC_TYPE_OPENSSL); + XFREE(caDer, NULL, DYNAMIC_TYPE_OPENSSL); + X509_free(signerCertX509); + X509_free(caX509); + BIO_free(signerCertBio); + BIO_free(caBio); +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_PKCS7_SIGNED_new(void) { EXPECT_DECLS; diff --git a/tests/api/test_ossl_p7p12.h b/tests/api/test_ossl_p7p12.h index 58aa9dd12a..6704ab51ed 100644 --- a/tests/api/test_ossl_p7p12.h +++ b/tests/api/test_ossl_p7p12.h @@ -27,6 +27,8 @@ int test_wolfssl_PKCS7(void); int test_wolfSSL_PKCS7_certs(void); int test_wolfSSL_PKCS7_sign(void); +int test_wolfSSL_PKCS7_verify_signer_forgery(void); +int test_wolfSSL_PKCS7_verify_sid_binding(void); int test_wolfSSL_PKCS7_SIGNED_new(void); int test_wolfSSL_PEM_write_bio_PKCS7(void); int test_wolfSSL_PEM_write_bio_encryptedKey(void); @@ -38,6 +40,8 @@ int test_wolfSSL_PKCS12(void); TEST_DECL_GROUP("ossl_p7", test_wolfssl_PKCS7), \ TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_certs), \ TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_sign), \ + TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_verify_signer_forgery), \ + TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_verify_sid_binding), \ TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PKCS7_SIGNED_new), \ TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PEM_write_bio_PKCS7), \ TEST_DECL_GROUP("ossl_p7", test_wolfSSL_PEM_write_bio_encryptedKey), \ diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 5d1a25bb31..2c1c3a80f0 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -97,6 +97,7 @@ typedef enum { /* holds information about the signers */ struct PKCS7SignerInfo { int version; + int sidType; /* CMS_ISSUER_AND_SERIAL_NUMBER or CMS_SKID */ byte *sid; word32 sidSz; }; @@ -4292,6 +4293,94 @@ int wc_PKCS7_SetEccSignRawDigestCb(wc_PKCS7* pkcs7, CallbackEccSignRawDigest cb) #endif /* HAVE_ECC */ +#if !defined(NO_RSA) || defined(HAVE_ECC) +/* Check whether the given decoded certificate matches the SignerIdentifier + * (sid) field of the currently parsed SignerInfo. Per RFC 5652 Section 5.3, + * the sid selects which certificate's public key must be used to verify the + * signature. Returns 1 on match, 0 on no match or when the sid is not + * available for comparison. */ +static int wc_PKCS7_CertMatchesSignerInfo(wc_PKCS7* pkcs7, DecodedCert* dCert) +{ + PKCS7SignerInfo* signerInfo; + + if (pkcs7 == NULL || dCert == NULL) + return 0; + + signerInfo = pkcs7->signerInfo; + if (signerInfo == NULL || signerInfo->sid == NULL || + signerInfo->sidSz == 0) { + /* No SID parsed, cannot perform an identity binding check. */ + return 0; + } + + if (signerInfo->sidType == CMS_ISSUER_AND_SERIAL_NUMBER) { + /* IssuerAndSerialNumber: SID blob stores the content of the outer + * SEQUENCE (issuer Name followed by INTEGER serialNumber). */ + word32 idx = 0; + byte sidIssuerHash[KEYID_SIZE]; + WC_DECLARE_VAR(sidSerial, mp_int, 1, pkcs7->heap); + WC_DECLARE_VAR(certSerial, mp_int, 1, pkcs7->heap); + int cmp; + int match = 0; + + if (GetNameHash_ex(signerInfo->sid, &idx, sidIssuerHash, + (int)signerInfo->sidSz, dCert->signatureOID) < 0) { + return 0; + } + if (XMEMCMP(sidIssuerHash, dCert->issuerHash, KEYID_SIZE) != 0) + return 0; + + WC_ALLOC_VAR_EX(sidSerial, mp_int, 1, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER, + { return 0; }); + WC_ALLOC_VAR_EX(certSerial, mp_int, 1, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER, + { WC_FREE_VAR_EX(sidSerial, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + return 0; }); + + if (mp_init(sidSerial) != MP_OKAY) { + WC_FREE_VAR_EX(sidSerial, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + WC_FREE_VAR_EX(certSerial, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + return 0; + } + if (mp_init(certSerial) != MP_OKAY) { + mp_clear(sidSerial); + WC_FREE_VAR_EX(sidSerial, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + WC_FREE_VAR_EX(certSerial, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + return 0; + } + + if (GetInt(sidSerial, signerInfo->sid, &idx, signerInfo->sidSz) == 0 && + mp_read_unsigned_bin(certSerial, dCert->serial, + (word32)dCert->serialSz) == MP_OKAY) { + cmp = mp_cmp(sidSerial, certSerial); + if (cmp == MP_EQ) + match = 1; + } + + mp_clear(sidSerial); + mp_clear(certSerial); + WC_FREE_VAR_EX(sidSerial, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + WC_FREE_VAR_EX(certSerial, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + return match; + } + else if (signerInfo->sidType == CMS_SKID) { + /* SubjectKeyIdentifier: SID blob is the raw SKID octet string + * content. Normalize the same way the certificate side does so + * that comparisons between SHA-1 SKIDs and other lengths match. */ + byte sidKid[KEYID_SIZE]; + + if (GetHashId(signerInfo->sid, (int)signerInfo->sidSz, sidKid, + HashIdAlg(dCert->signatureOID)) != 0) { + return 0; + } + if (XMEMCMP(sidKid, dCert->extSubjKeyId, KEYID_SIZE) == 0) + return 1; + return 0; + } + + return 0; +} +#endif /* !NO_RSA || HAVE_ECC */ + #ifndef NO_RSA /* returns size of signature put into out, negative on error */ @@ -4371,6 +4460,31 @@ static int wc_PKCS7_RsaVerify(wc_PKCS7* pkcs7, byte* sig, int sigSz, continue; } + /* If the SignerInfo sid was parsed, only try the certificate whose + * identity matches it. This binds the verifying public key to the + * signer identity advertised in the CMS message and prevents signer + * confusion when multiple certificates are embedded. */ + if (pkcs7->signerInfo != NULL && pkcs7->signerInfo->sid != NULL && + !wc_PKCS7_CertMatchesSignerInfo(pkcs7, dCert)) { + FreeDecodedCert(dCert); + wc_FreeRsaKey(key); + continue; + } + + /* Defense in depth: the sid-matched cert must actually carry an + * RSA-family key before we feed its SPKI to the RSA key decoder. + * Rejecting here avoids depending on wc_RsaPublicKeyDecode to reject + * wrong-type SPKIs. */ + if (dCert->keyOID != RSAk + #ifdef WC_RSA_PSS + && dCert->keyOID != RSAPSSk + #endif + ) { + FreeDecodedCert(dCert); + wc_FreeRsaKey(key); + continue; + } + if (wc_RsaPublicKeyDecode(dCert->publicKey, &scratch, key, dCert->pubKeySize) < 0) { WOLFSSL_MSG("ASN RSA key decode error"); @@ -4481,6 +4595,24 @@ static int wc_PKCS7_RsaPssVerify(wc_PKCS7* pkcs7, byte* sig, int sigSz, continue; } + /* Only try the certificate identified by the SignerInfo sid (see + * matching comment in wc_PKCS7_RsaVerify). */ + if (pkcs7->signerInfo != NULL && pkcs7->signerInfo->sid != NULL && + !wc_PKCS7_CertMatchesSignerInfo(pkcs7, dCert)) { + FreeDecodedCert(dCert); + wc_FreeRsaKey(key); + continue; + } + + /* Defense in depth: reject non-RSA SPKIs before key decode. RSA + * rsaEncryption certs (keyOID=RSAk) are accepted for PSS signatures + * per RFC 8017 - a RSASSA-PSS cert is not required. */ + if (dCert->keyOID != RSAk && dCert->keyOID != RSAPSSk) { + FreeDecodedCert(dCert); + wc_FreeRsaKey(key); + continue; + } + pkSz = dCert->pubKeySize; if (pkSz > (MAX_RSA_INT_SZ + MAX_RSA_E_SZ)) pkSz = (MAX_RSA_INT_SZ + MAX_RSA_E_SZ); @@ -4646,6 +4778,22 @@ static int wc_PKCS7_EcdsaVerify(wc_PKCS7* pkcs7, byte* sig, int sigSz, continue; } + /* Only try the certificate identified by the SignerInfo sid (see + * matching comment in wc_PKCS7_RsaVerify). */ + if (pkcs7->signerInfo != NULL && pkcs7->signerInfo->sid != NULL && + !wc_PKCS7_CertMatchesSignerInfo(pkcs7, dCert)) { + FreeDecodedCert(dCert); + wc_ecc_free(key); + continue; + } + + /* Defense in depth: reject non-ECDSA SPKIs before key decode. */ + if (dCert->keyOID != ECDSAk) { + FreeDecodedCert(dCert); + wc_ecc_free(key); + continue; + } + if (wc_EccPublicKeyDecode(dCert->publicKey, &idx, key, dCert->pubKeySize) < 0) { WOLFSSL_MSG("ASN ECC key decode error"); @@ -5359,11 +5507,16 @@ static int wc_PKCS7_ParseSignerInfo(wc_PKCS7* pkcs7, byte* in, word32 inSz, ret = ASN_PARSE_E; if (ret == 0) { + pkcs7->signerInfo->sidType = CMS_ISSUER_AND_SERIAL_NUMBER; ret = wc_PKCS7_SignerInfoSetSID(pkcs7, in + idx, length); idx += (word32)length; } } else if (ret == 0 && version == 3) { + /* Default: SignerInfo version 3 carries SubjectKeyIdentifier. + * May be overridden below if the parser instead finds a + * SEQUENCE (IssuerAndSerialNumber fallback). */ + pkcs7->signerInfo->sidType = CMS_SKID; /* Get the sequence of SubjectKeyIdentifier */ if (idx + 1 > inSz) ret = BUFFER_E; @@ -5406,6 +5559,12 @@ static int wc_PKCS7_ParseSignerInfo(wc_PKCS7* pkcs7, byte* in, word32 inSz, if (ret == 0 && GetSequence(in, &idx, &length, inSz) < 0) ret = ASN_PARSE_E; + + if (ret == 0) { + /* v3 carrying IssuerAndSerialNumber fallback */ + pkcs7->signerInfo->sidType = + CMS_ISSUER_AND_SERIAL_NUMBER; + } } } From 97b82b50873e16feb90b9c911424d3ed547ee36e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Tue, 21 Apr 2026 18:33:45 +0200 Subject: [PATCH 10/11] Add nonce length validation for PKCS#7 --- wolfcrypt/src/pkcs7.c | 45 ++++++++++++++++++++++++++++++++++ wolfssl/wolfcrypt/wc_encrypt.h | 3 +++ 2 files changed, 48 insertions(+) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 2c1c3a80f0..3b34dd970c 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -14726,6 +14726,11 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in, break; } pkiMsgSz = (pkcs7->stream->length > 0)? pkcs7->stream->length: inSz; + + /* Restore encOID across WANT_READ re-entries so the nonce + * length validation below always sees the content-cipher + * algorithm parsed in AUTHENV_3. */ + wc_PKCS7_StreamGetVar(pkcs7, &encOID, &blockKeySz, NULL); #endif /* get length of optional parameter sequence */ if (ret == 0 && GetLength(pkiMsg, &idx, &length, pkiMsgSz) < 0) { @@ -14743,6 +14748,46 @@ int wc_PKCS7_DecodeAuthEnvelopedData(wc_PKCS7* pkcs7, byte* in, ret = ASN_PARSE_E; } + /* Enforce algorithm-specific nonce length bounds at the parser + * layer so malformed lengths (notably zero-length, which would + * catastrophically break AEAD uniqueness on HW backends that + * skip their own checks) cannot reach the cipher engine. + * - AES-GCM in CMS: RFC 5084 Sec. 3.2 mandates a 12-octet IV. + * - AES-CCM: RFC 3610 Sec. 2.3 requires 7..13 octets. + * Any other encOID here is a parser-state invariant violation. */ + if (ret == 0) { + int nonceMin = 0, nonceMax = 0; + switch (encOID) { + #ifdef HAVE_AESGCM + case AES128GCMb: + case AES192GCMb: + case AES256GCMb: + nonceMin = GCM_NONCE_MID_SZ; + nonceMax = GCM_NONCE_MID_SZ; + break; + #endif + #ifdef HAVE_AESCCM + case AES128CCMb: + case AES192CCMb: + case AES256CCMb: + nonceMin = CCM_NONCE_MIN_SZ; + nonceMax = CCM_NONCE_MAX_SZ; + break; + #endif + default: + WOLFSSL_MSG( + "AuthEnvelopedData unexpected content cipher"); + ret = ALGO_ID_E; + break; + } + if (ret == 0 && + (nonceSz < nonceMin || nonceSz > nonceMax)) { + WOLFSSL_MSG( + "AuthEnvelopedData nonce length invalid for cipher"); + ret = ASN_PARSE_E; + } + } + if (ret == 0) { XMEMCPY(nonce, &pkiMsg[idx], (word32)nonceSz); idx += (word32)nonceSz; diff --git a/wolfssl/wolfcrypt/wc_encrypt.h b/wolfssl/wolfcrypt/wc_encrypt.h index da11b53922..7f1c48b636 100644 --- a/wolfssl/wolfcrypt/wc_encrypt.h +++ b/wolfssl/wolfcrypt/wc_encrypt.h @@ -73,6 +73,9 @@ #ifndef CCM_NONCE_MIN_SZ #define CCM_NONCE_MIN_SZ 7 #endif + #ifndef CCM_NONCE_MAX_SZ + #define CCM_NONCE_MAX_SZ 13 + #endif #endif From 22d14413311a6d0bb55221a2e4c9759f9530651a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Frauenschl=C3=A4ger?= Date: Thu, 23 Apr 2026 10:19:36 +0200 Subject: [PATCH 11/11] Bounds-check the RecipientInfo SET length in wc_PKCS7_ParseToRecipientInfoSet() --- tests/api/test_pkcs7.c | 66 ++++++++++++++++++++++++++++++++++++++++++ tests/api/test_pkcs7.h | 4 ++- wolfcrypt/src/pkcs7.c | 37 +++++++++++++++++++++++ 3 files changed, 106 insertions(+), 1 deletion(-) diff --git a/tests/api/test_pkcs7.c b/tests/api/test_pkcs7.c index a22fd9e10e..69b4631605 100644 --- a/tests/api/test_pkcs7.c +++ b/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 */ diff --git a/tests/api/test_pkcs7.h b/tests/api/test_pkcs7.h index e1c71c911d..f4aed161da 100644 --- a/tests/api/test_pkcs7.h +++ b/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) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 3b34dd970c..666890b608 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/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;