From fcb13ed3914d6ee2b9f084f38c3bc54034555aab Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Thu, 30 Jul 2026 12:31:03 +0900 Subject: [PATCH 1/7] fix f7302 pkcs7 --- tests/api/test_pkcs7.c | 197 ++++++++++++++++++++++++++++++++++++++--- tests/api/test_pkcs7.h | 6 ++ wolfcrypt/src/pkcs7.c | 33 ++++--- 3 files changed, 208 insertions(+), 28 deletions(-) diff --git a/tests/api/test_pkcs7.c b/tests/api/test_pkcs7.c index 95c5f6ed3e2..767bf0b6f8e 100644 --- a/tests/api/test_pkcs7.c +++ b/tests/api/test_pkcs7.c @@ -6285,19 +6285,15 @@ int test_wc_PKCS7_VerifySignedData_TruncEContentTag(void) /* * SignedData bundle truncated at the certificates [0] IMPLICIT tag. * Verifies that the parser rejects the malformed input rather than - * dereferencing past the end of the buffer. - * - * TODO: limited to NO_PKCS7_STREAM because the streaming parser's stage 3 - * early-exit check (pkcs7.c near line 6594) accepts any bundle - * whose remaining footer is < 6 bytes as a successful degenerate end, - * so the bounds check at line 6765 is unreachable in streaming mode. - * Drop the NO_PKCS7_STREAM gate if/when the early-exit check becomes - * more accurate. + * dereferencing past the end of the buffer. Runs in both streaming and + * NO_PKCS7_STREAM builds: the streaming parser's stage 3 early-exit check + * used to accept any bundle whose remaining footer was < 6 bytes as a + * successful degenerate end, silently accepting this truncated input. */ int test_wc_PKCS7_VerifySignedData_TruncCertSetTag(void) { EXPECT_DECLS; -#if defined(HAVE_PKCS7) && defined(NO_PKCS7_STREAM) +#if defined(HAVE_PKCS7) PKCS7* pkcs7 = NULL; WOLFSSL_SMALL_STACK_STATIC byte der[] = { @@ -6337,7 +6333,188 @@ int test_wc_PKCS7_VerifySignedData_TruncCertSetTag(void) ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), 0); wc_PKCS7_Free(pkcs7); -#endif /* HAVE_PKCS7 && NO_PKCS7_STREAM */ +#endif /* HAVE_PKCS7 */ + return EXPECT_RESULT(); +} + +/* + * SignedData bundle that is a genuine, non-truncated degenerate + * (certs-only) bundle: no certificates, no CRLs, and an empty signerInfos + * SET ("31 00") closing the bundle right after the content. This is the + * shortest legitimate ending the stage 3 tail check in the streaming + * parser can see, and must still succeed after fixing that check to reject + * truncated bundles. + */ +int test_wc_PKCS7_VerifySignedData_DegenerateMinimal(void) +{ + EXPECT_DECLS; +#if defined(HAVE_PKCS7) + PKCS7* pkcs7 = NULL; + + WOLFSSL_SMALL_STACK_STATIC byte der[] = { + /* outer ContentInfo SEQUENCE (99 bytes content) */ + 0x30, 0x63, + /* contentType OID signedData */ + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x02, + /* [0] EXPLICIT (86 bytes content) */ + 0xA0, 0x56, + /* SignedData SEQUENCE (84 bytes content) */ + 0x30, 0x54, + /* version INTEGER 1 */ + 0x02, 0x01, 0x01, + /* digestAlgorithms SET (empty - degenerate) */ + 0x31, 0x00, + /* encapContentInfo SEQUENCE (75 bytes content) */ + 0x30, 0x4B, + /* eContentType OID 1.2.840.113549.1.7.1 (data) */ + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x01, + /* eContent [0] EXPLICIT (62 bytes content) */ + 0xA0, 0x3E, + /* OCTET STRING (60 bytes content) */ + 0x04, 0x3C, + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, 0x1E, 0x1F, + 0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, + 0x28, 0x29, 0x2A, 0x2B, 0x2C, 0x2D, 0x2E, 0x2F, + 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, + 0x38, 0x39, 0x3A, 0x3B, + /* signerInfos SET (empty - degenerate end) */ + 0x31, 0x00 + }; + word32 derSz = (word32)sizeof(der); + + /* single-shot call */ + ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); + ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); + ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); + ExpectIntEQ(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), 0); + if (pkcs7 != NULL) { + ExpectIntEQ(pkcs7->contentSz, 60); + ExpectNotNull(pkcs7->content); + } + wc_PKCS7_Free(pkcs7); + pkcs7 = NULL; + +#ifndef NO_PKCS7_STREAM + /* same bundle fed one byte at a time */ + { + int ret; + word32 idx; + + ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); + ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); + ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); + + ret = WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E); + for (idx = 0; idx < derSz && ret != 0; idx++) { + ret = wc_PKCS7_VerifySignedData(pkcs7, der + idx, 1); + if (ret < 0 && ret != WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)) { + break; + } + } + ExpectIntEQ(ret, 0); + if (pkcs7 != NULL) { + ExpectIntEQ(pkcs7->contentSz, 60); + ExpectNotNull(pkcs7->content); + } + wc_PKCS7_Free(pkcs7); + } +#endif /* !NO_PKCS7_STREAM */ + +#endif /* HAVE_PKCS7 */ + return EXPECT_RESULT(); +} + +/* + * Same shape as test_wc_PKCS7_VerifySignedData_DegenerateMinimal, but the + * final 2 bytes are "31 01" instead of "31 00": a signerInfos SET claiming + * one byte of content that the buffer never supplies. Must be rejected, + * not treated as a successful degenerate end. + */ +int test_wc_PKCS7_VerifySignedData_TruncSignerInfosTag(void) +{ + EXPECT_DECLS; +#if defined(HAVE_PKCS7) + PKCS7* pkcs7 = NULL; + + WOLFSSL_SMALL_STACK_STATIC byte der[] = { + 0x30, 0x63, + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x02, + 0xA0, 0x56, + 0x30, 0x54, + 0x02, 0x01, 0x01, + 0x31, 0x00, + 0x30, 0x4B, + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x01, + 0xA0, 0x3E, + 0x04, 0x3C, + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, 0x1E, 0x1F, + 0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, + 0x28, 0x29, 0x2A, 0x2B, 0x2C, 0x2D, 0x2E, 0x2F, + 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, + 0x38, 0x39, 0x3A, 0x3B, + /* signerInfos SET claims 1 byte of content, buffer ends here */ + 0x31, 0x01 + }; + word32 derSz = (word32)sizeof(der); + + ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); + ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); + ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); + ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), 0); + wc_PKCS7_Free(pkcs7); + +#endif /* HAVE_PKCS7 */ + return EXPECT_RESULT(); +} + +/* + * Same shape again, but the signerInfos SET is missing entirely: the + * buffer ends right after the content, with nothing following. signerInfos + * is a mandatory field, so this must be rejected rather than accepted as + * a bundle with no more elements. + */ +int test_wc_PKCS7_VerifySignedData_NoSignerInfosTag(void) +{ + EXPECT_DECLS; +#if defined(HAVE_PKCS7) + PKCS7* pkcs7 = NULL; + + WOLFSSL_SMALL_STACK_STATIC byte der[] = { + 0x30, 0x61, + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x02, + 0xA0, 0x54, + 0x30, 0x52, + 0x02, 0x01, 0x01, + 0x31, 0x00, + 0x30, 0x4B, + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x01, + 0xA0, 0x3E, + 0x04, 0x3C, + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, 0x1E, 0x1F, + 0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, + 0x28, 0x29, 0x2A, 0x2B, 0x2C, 0x2D, 0x2E, 0x2F, + 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, + 0x38, 0x39, 0x3A, 0x3B + /* buffer ends here -- no signerInfos SET at all */ + }; + word32 derSz = (word32)sizeof(der); + + ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); + ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); + ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); + ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), 0); + wc_PKCS7_Free(pkcs7); + +#endif /* HAVE_PKCS7 */ return EXPECT_RESULT(); } diff --git a/tests/api/test_pkcs7.h b/tests/api/test_pkcs7.h index ed2a036afaa..7161a7104a2 100644 --- a/tests/api/test_pkcs7.h +++ b/tests/api/test_pkcs7.h @@ -78,6 +78,9 @@ int test_wc_PKCS7_VerifySignedData_PKCS7ContentSeq(void); int test_wc_PKCS7_VerifySignedData_IndefLenOOB(void); int test_wc_PKCS7_VerifySignedData_TruncEContentTag(void); int test_wc_PKCS7_VerifySignedData_TruncCertSetTag(void); +int test_wc_PKCS7_VerifySignedData_DegenerateMinimal(void); +int test_wc_PKCS7_VerifySignedData_TruncSignerInfosTag(void); +int test_wc_PKCS7_VerifySignedData_NoSignerInfosTag(void); int test_wc_PKCS7_VerifySignedData_NoDigestParams(void); @@ -133,6 +136,9 @@ int test_wc_PKCS7_VerifySignedData_NoDigestParams(void); TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_IndefLenOOB), \ TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_TruncEContentTag), \ TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_TruncCertSetTag), \ + TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_DegenerateMinimal), \ + TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_TruncSignerInfosTag), \ + TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_NoSignerInfosTag), \ TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_NoDigestParams) #define TEST_PKCS7_ENCRYPTED_DATA_DECLS \ diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 546d79535ab..a6265d3b61d 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -7506,26 +7506,23 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf, pkcs7->content = pkcs7->contentDynamic; } - /* check if bundle has more elements or footer, if not, set content - * to pkcs7->content and hash to pkcs7->hash. - * - * NOTE: this check returns success whenever fewer than 6 bytes - * follow the content within the outer ContentInfo, which also - * accepts truncated bundles whose footer was cut short (e.g. a - * lone certificates [0] tag with no length). Distinguishing a - * legitimate degenerate end (such as an empty signerInfos SET - * "31 00") from truncated junk would require peeking at the - * remaining bytes or making stage 4's `expected` window smaller. + /* expect data length to be enough to check set and seq of certs, + * but never request more than the bundle has left. The old code + * unconditionally requested this much and silently treated a + * short read here as a successful degenerate end, which also + * accepted a truncated bundle. Capping the request lets stage + * 4/5/6's existing bounds checks and noDegenerate enforcement + * run instead: a genuine short degenerate end (empty signerInfos + * SET) still parses and succeeds, while anything truncated or + * malformed fails there with a real parse error. */ - if (ret == 0 && pkcs7->stream->maxLen > 0 && - (pkcs7->stream->maxLen - pkcs7->stream->totalRd) - < ASN_TAG_SZ + MAX_LENGTH_SZ) { - - ret = 0; - break; - } - /* expect data length to be enough to check set and seq of certs */ pkcs7->stream->expected = (ASN_TAG_SZ + MAX_LENGTH_SZ) * 2; + if (pkcs7->stream->totalRd < pkcs7->stream->maxLen && + pkcs7->stream->expected > + pkcs7->stream->maxLen - pkcs7->stream->totalRd) { + pkcs7->stream->expected = + pkcs7->stream->maxLen - pkcs7->stream->totalRd; + } #else /* Break out before content because it can be optional in degenerate From 72c4aa5280604b36600bbca778018f980e780530 Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Thu, 30 Jul 2026 16:25:44 +0900 Subject: [PATCH 2/7] additional fix --- tests/api/test_pkcs7.c | 65 ++++++++++++++++++++++++++++++++++++++++++ tests/api/test_pkcs7.h | 2 ++ wolfcrypt/src/pkcs7.c | 2 +- 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/tests/api/test_pkcs7.c b/tests/api/test_pkcs7.c index 767bf0b6f8e..89f7aac7f0f 100644 --- a/tests/api/test_pkcs7.c +++ b/tests/api/test_pkcs7.c @@ -6518,6 +6518,71 @@ int test_wc_PKCS7_VerifySignedData_NoSignerInfosTag(void) return EXPECT_RESULT(); } +/* + * SignedData bundle that is well-formed and NOT truncated: digestAlgorithms + * SET contains one real AlgorithmIdentifier (so the early heuristic that + * flags a bundle as degenerate from an empty digestAlgorithms SET does not + * fire), while signerInfos SET is genuinely empty (degenerate, no signer). + * With wc_PKCS7_AllowDegenerate(pkcs7, 0) set, this must be rejected once + * the accurate signerInfos-based degenerate determination runs, not + * silently accepted because the early heuristic missed it. + */ +int test_wc_PKCS7_VerifySignedData_DegenerateNonEmptyDigestAlgos(void) +{ + EXPECT_DECLS; +#if defined(HAVE_PKCS7) + PKCS7* pkcs7 = NULL; + + WOLFSSL_SMALL_STACK_STATIC byte der[] = { + /* outer ContentInfo SEQUENCE (114 bytes content) */ + 0x30, 0x72, + /* contentType OID signedData */ + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x02, + /* [0] EXPLICIT (101 bytes content) */ + 0xA0, 0x65, + /* SignedData SEQUENCE (99 bytes content) */ + 0x30, 0x63, + /* version INTEGER 1 */ + 0x02, 0x01, 0x01, + /* digestAlgorithms SET (15 bytes content) -- one real + * AlgorithmIdentifier (sha256), not empty */ + 0x31, 0x0F, + 0x30, 0x0D, + 0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01, + 0x05, 0x00, + /* encapContentInfo SEQUENCE (75 bytes content) */ + 0x30, 0x4B, + /* eContentType OID 1.2.840.113549.1.7.1 (data) */ + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x01, + /* eContent [0] EXPLICIT (62 bytes content) */ + 0xA0, 0x3E, + /* OCTET STRING (60 bytes content) */ + 0x04, 0x3C, + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, 0x1E, 0x1F, + 0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, + 0x28, 0x29, 0x2A, 0x2B, 0x2C, 0x2D, 0x2E, 0x2F, + 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, + 0x38, 0x39, 0x3A, 0x3B, + /* signerInfos SET (empty -- genuinely degenerate) */ + 0x31, 0x00 + }; + word32 derSz = (word32)sizeof(der); + + ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); + ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); + ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); + wc_PKCS7_AllowDegenerate(pkcs7, 0); + ExpectIntEQ(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), + PKCS7_NO_SIGNER_E); + wc_PKCS7_Free(pkcs7); + +#endif /* HAVE_PKCS7 */ + return EXPECT_RESULT(); +} + #if defined(HAVE_PKCS7) && !defined(NO_RSA) && !defined(NO_SHA256) && \ defined(USE_CERT_BUFFERS_2048) /* diff --git a/tests/api/test_pkcs7.h b/tests/api/test_pkcs7.h index 7161a7104a2..cd0c9a8c1a3 100644 --- a/tests/api/test_pkcs7.h +++ b/tests/api/test_pkcs7.h @@ -81,6 +81,7 @@ int test_wc_PKCS7_VerifySignedData_TruncCertSetTag(void); int test_wc_PKCS7_VerifySignedData_DegenerateMinimal(void); int test_wc_PKCS7_VerifySignedData_TruncSignerInfosTag(void); int test_wc_PKCS7_VerifySignedData_NoSignerInfosTag(void); +int test_wc_PKCS7_VerifySignedData_DegenerateNonEmptyDigestAlgos(void); int test_wc_PKCS7_VerifySignedData_NoDigestParams(void); @@ -139,6 +140,7 @@ int test_wc_PKCS7_VerifySignedData_NoDigestParams(void); TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_DegenerateMinimal), \ TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_TruncSignerInfosTag), \ TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_NoSignerInfosTag), \ + TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_DegenerateNonEmptyDigestAlgos), \ TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_NoDigestParams) #define TEST_PKCS7_ENCRYPTED_DATA_DECLS \ diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index a6265d3b61d..6f28382c3e0 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -6391,7 +6391,7 @@ static int wc_PKCS7_ParseSignerInfo(wc_PKCS7* pkcs7, byte* in, word32 inSz, WOLFSSL_ENTER("wc_PKCS7_ParseSignerInfo"); /* require a signer if degenerate case not allowed */ - if (inSz == 0 && pkcs7->noDegenerate == 1) { + if (pkcs7->noDegenerate == 1 && (inSz == 0 || degenerate == 1)) { WOLFSSL_MSG("Set to not allow degenerate cases"); return PKCS7_NO_SIGNER_E; } From a600695d8a063baec4226f499fe8b46ee3302cea Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Fri, 31 Jul 2026 17:59:14 +0900 Subject: [PATCH 3/7] tests: remove bare scope block in PKCS7 DegenerateMinimal stream test Hoist ret/idx declarations to the top of the HAVE_PKCS7 block instead of a standalone { ... } scope, per wolfSSL C coding style. --- tests/api/test_pkcs7.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/api/test_pkcs7.c b/tests/api/test_pkcs7.c index 89f7aac7f0f..ee2e973cfe2 100644 --- a/tests/api/test_pkcs7.c +++ b/tests/api/test_pkcs7.c @@ -6350,6 +6350,10 @@ int test_wc_PKCS7_VerifySignedData_DegenerateMinimal(void) EXPECT_DECLS; #if defined(HAVE_PKCS7) PKCS7* pkcs7 = NULL; +#ifndef NO_PKCS7_STREAM + int ret; + word32 idx; +#endif WOLFSSL_SMALL_STACK_STATIC byte der[] = { /* outer ContentInfo SEQUENCE (99 bytes content) */ @@ -6399,10 +6403,6 @@ int test_wc_PKCS7_VerifySignedData_DegenerateMinimal(void) #ifndef NO_PKCS7_STREAM /* same bundle fed one byte at a time */ - { - int ret; - word32 idx; - ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); @@ -6420,7 +6420,6 @@ int test_wc_PKCS7_VerifySignedData_DegenerateMinimal(void) ExpectNotNull(pkcs7->content); } wc_PKCS7_Free(pkcs7); - } #endif /* !NO_PKCS7_STREAM */ #endif /* HAVE_PKCS7 */ From 8777c2b4707205dbd6c3de6444653cd424c3e109 Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Fri, 31 Jul 2026 17:59:23 +0900 Subject: [PATCH 4/7] pkcs7: fix VerifySignedData stall when signerInfos is missing entirely Stage 3's cap on stream->expected only applied while totalRd < maxLen. When a bundle is truncated exactly at the outer SignedData length (0 bytes remain, totalRd == maxLen), the cap did not fire, leaving expected at a fixed non-zero value. Stage 4 then requested bytes that would never arrive, returning WC_PKCS7_WANT_READ_E instead of a deterministic parse error, in both single-shot and streaming calls. Cap expected to 0 when totalRd >= maxLen so stage 4's existing bounds check fires immediately with BUFFER_E. Strengthen test_wc_PKCS7_VerifySignedData_NoSignerInfosTag to assert the return code is not WC_PKCS7_WANT_READ_E, and add a streaming byte-at-a-time variant, since the previous ExpectIntNE(ret, 0) check did not catch the stall. --- tests/api/test_pkcs7.c | 28 ++++++++++++++++++++++++++++ wolfcrypt/src/pkcs7.c | 9 +++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/tests/api/test_pkcs7.c b/tests/api/test_pkcs7.c index ee2e973cfe2..7d4f266a157 100644 --- a/tests/api/test_pkcs7.c +++ b/tests/api/test_pkcs7.c @@ -6483,6 +6483,10 @@ int test_wc_PKCS7_VerifySignedData_NoSignerInfosTag(void) EXPECT_DECLS; #if defined(HAVE_PKCS7) PKCS7* pkcs7 = NULL; +#ifndef NO_PKCS7_STREAM + int ret; + word32 idx; +#endif WOLFSSL_SMALL_STACK_STATIC byte der[] = { 0x30, 0x61, @@ -6507,11 +6511,35 @@ int test_wc_PKCS7_VerifySignedData_NoSignerInfosTag(void) }; word32 derSz = (word32)sizeof(der); + /* single-shot: must fail with a real parse error, not WANT_READ_E + * (no more bytes will ever arrive per the outer length) */ ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), 0); + ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), + WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)); + wc_PKCS7_Free(pkcs7); + pkcs7 = NULL; + +#ifndef NO_PKCS7_STREAM + /* same bundle fed one byte at a time: must not end stuck on + * WANT_READ_E once all available bytes are consumed */ + ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); + ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); + ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); + + ret = WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E); + for (idx = 0; idx < derSz && ret != 0; idx++) { + ret = wc_PKCS7_VerifySignedData(pkcs7, der + idx, 1); + if (ret < 0 && ret != WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)) { + break; + } + } + ExpectIntNE(ret, 0); + ExpectIntNE(ret, WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)); wc_PKCS7_Free(pkcs7); +#endif /* !NO_PKCS7_STREAM */ #endif /* HAVE_PKCS7 */ return EXPECT_RESULT(); diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 6f28382c3e0..9cb6f56739c 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -7517,8 +7517,13 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf, * malformed fails there with a real parse error. */ pkcs7->stream->expected = (ASN_TAG_SZ + MAX_LENGTH_SZ) * 2; - if (pkcs7->stream->totalRd < pkcs7->stream->maxLen && - pkcs7->stream->expected > + if (pkcs7->stream->totalRd >= pkcs7->stream->maxLen) { + /* no bytes left per the outer length: force stage 4's + * bounds check to fail now instead of requesting bytes + * that will never arrive. */ + pkcs7->stream->expected = 0; + } + else if (pkcs7->stream->expected > pkcs7->stream->maxLen - pkcs7->stream->totalRd) { pkcs7->stream->expected = pkcs7->stream->maxLen - pkcs7->stream->totalRd; From 59c4c70872f7f1be40be2bce3350ec16995c4fba Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Sat, 1 Aug 2026 06:13:32 +0900 Subject: [PATCH 5/7] pkcs7: don't zero-cap stage 3 expected for indefinite-length bundles The totalRd >= maxLen zero-cap added in 8777c2b47 also fired for BER indefinite-length bundles, where maxLen is only a running estimate set from the first available bytes and grows as parsing progresses, not a hard bound on the message size. That made totalRd catch up to maxLen mid-stream on legitimate encode/verify round trips, turning a normal WC_PKCS7_WANT_READ_E into a premature BUFFER_E/ASN_PARSE_E and breaking --enable-all make check (test_wc_PKCS7_EncodeSignedData, test_wc_PKCS7_VerifySignedData_RSA, test_wc_PKCS7_BER). Only apply the zero-cap when pkcs7->stream->indefLen is not set, so it's limited to definite-length bundles where maxLen is authoritative. --- wolfcrypt/src/pkcs7.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 9cb6f56739c..ce373e0f054 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -7517,13 +7517,18 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf, * malformed fails there with a real parse error. */ pkcs7->stream->expected = (ASN_TAG_SZ + MAX_LENGTH_SZ) * 2; - if (pkcs7->stream->totalRd >= pkcs7->stream->maxLen) { - /* no bytes left per the outer length: force stage 4's - * bounds check to fail now instead of requesting bytes - * that will never arrive. */ + if (!pkcs7->stream->indefLen && + pkcs7->stream->totalRd >= pkcs7->stream->maxLen) { + /* definite-length bundle with no bytes left per the outer + * length: force stage 4's bounds check to fail now instead + * of requesting bytes that will never arrive. For + * indefinite-length (BER) bundles maxLen is only a running + * estimate, so totalRd catching up to it does not mean the + * bundle is actually exhausted. */ pkcs7->stream->expected = 0; } - else if (pkcs7->stream->expected > + else if (pkcs7->stream->totalRd < pkcs7->stream->maxLen && + pkcs7->stream->expected > pkcs7->stream->maxLen - pkcs7->stream->totalRd) { pkcs7->stream->expected = pkcs7->stream->maxLen - pkcs7->stream->totalRd; From 472ec625897737075d5fb7147b528aa2b7c9197d Mon Sep 17 00:00:00 2001 From: HIDEKI MIYAZAKI Date: Wed, 5 Aug 2026 17:55:17 +0900 Subject: [PATCH 6/7] Addressed review comments --- tests/api/test_pkcs7.c | 2 ++ wolfcrypt/src/pkcs7.c | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/tests/api/test_pkcs7.c b/tests/api/test_pkcs7.c index 7d4f266a157..c29c5ce4c48 100644 --- a/tests/api/test_pkcs7.c +++ b/tests/api/test_pkcs7.c @@ -6466,6 +6466,8 @@ int test_wc_PKCS7_VerifySignedData_TruncSignerInfosTag(void) ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), 0); + ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), + WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)); wc_PKCS7_Free(pkcs7); #endif /* HAVE_PKCS7 */ diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index ce373e0f054..bb566cf1aeb 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -8129,6 +8129,18 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf, } else { pkcs7->stream->expected = (word32)length; + if (pkcs7->stream->totalRd >= pkcs7->stream->maxLen) { + /* definite-length bundle with no bytes left per the + * outer length: force stage 7's bounds check to fail + * now instead of requesting bytes that will never + * arrive. Do NOT cap expected down further than that + * when bytes may still remain -- expected here is the + * full signerInfo content size that stage 7 needs + * buffered at once, not just header lookahead, so + * shrinking it while genuinely more data is still due + * from a future call would corrupt the parse window. */ + pkcs7->stream->expected = 0; + } } wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_VERIFY_STAGE7); From ffcd3e2c3785bd4a24b305b63062eeab484fe074 Mon Sep 17 00:00:00 2001 From: Hideki Miyazaki Date: Fri, 7 Aug 2026 23:42:21 +0900 Subject: [PATCH 7/7] Addressed Skoll comments one more --- tests/api/test_pkcs7.c | 235 ++++++++++++++++++++++++++++++++++------- tests/api/test_pkcs7.h | 4 + wolfcrypt/src/pkcs7.c | 88 ++++++++++----- 3 files changed, 260 insertions(+), 67 deletions(-) diff --git a/tests/api/test_pkcs7.c b/tests/api/test_pkcs7.c index c29c5ce4c48..d6a9367e116 100644 --- a/tests/api/test_pkcs7.c +++ b/tests/api/test_pkcs7.c @@ -6234,6 +6234,64 @@ int test_wc_PKCS7_VerifySignedData_IndefLenOOB(void) return EXPECT_RESULT(); } +#if defined(HAVE_PKCS7) && !defined(NO_PKCS7_STREAM) +/* + * Feeds der to wc_PKCS7_VerifySignedData at every chunk size from + * minChunkSz to derSz. If expectContentSz > 0, every chunk size must + * decode successfully and produce that content size; otherwise every + * chunk size must fail with a real parse error and never leave the + * result at WC_PKCS7_WANT_READ_E once the whole buffer has been fed. + */ +static int test_wc_PKCS7_VerifySignedData_ChunkSweep_once(const byte* der, + word32 derSz, word32 minChunkSz, word32 expectContentSz) +{ + EXPECT_DECLS; + PKCS7* pkcs7 = NULL; + int ret; + word32 chunkSz; + word32 off; + word32 thisSz; + word32 fed; + + for (chunkSz = minChunkSz; chunkSz <= derSz; chunkSz++) { + ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); + ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); + ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); + + fed = 0; + ret = WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E); + for (off = 0; off < derSz && ret != 0; off += chunkSz) { + thisSz = min(chunkSz, derSz - off); + ret = wc_PKCS7_VerifySignedData(pkcs7, (byte*)der + off, thisSz); + fed = off + thisSz; + if (ret < 0 && ret != WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)) { + break; + } + } + + if (expectContentSz > 0) { + ExpectIntEQ(ret, 0); + /* a genuine success must only happen once every byte of the + * bundle has actually been fed in; an early success here + * means the parser accepted a truncated prefix */ + ExpectIntEQ(fed, derSz); + if (pkcs7 != NULL) { + ExpectIntEQ(pkcs7->contentSz, expectContentSz); + ExpectNotNull(pkcs7->content); + } + } + else { + ExpectIntNE(ret, 0); + ExpectIntNE(ret, WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)); + } + wc_PKCS7_Free(pkcs7); + pkcs7 = NULL; + } + + return EXPECT_RESULT(); +} +#endif /* HAVE_PKCS7 && !NO_PKCS7_STREAM */ + /* * SignedData bundle truncated at the eContent [0] EXPLICIT tag in * encapContentInfo. Verifies that the parser rejects the malformed @@ -6278,6 +6336,11 @@ int test_wc_PKCS7_VerifySignedData_TruncEContentTag(void) ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), 0); wc_PKCS7_Free(pkcs7); +#ifndef NO_PKCS7_STREAM + EXPECT_TEST(test_wc_PKCS7_VerifySignedData_ChunkSweep_once(der, derSz, + 1, 0)); +#endif + #endif /* HAVE_PKCS7 */ return EXPECT_RESULT(); } @@ -6333,6 +6396,11 @@ int test_wc_PKCS7_VerifySignedData_TruncCertSetTag(void) ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), 0); wc_PKCS7_Free(pkcs7); +#ifndef NO_PKCS7_STREAM + EXPECT_TEST(test_wc_PKCS7_VerifySignedData_ChunkSweep_once(der, derSz, + 1, 0)); +#endif + #endif /* HAVE_PKCS7 */ return EXPECT_RESULT(); } @@ -6350,10 +6418,6 @@ int test_wc_PKCS7_VerifySignedData_DegenerateMinimal(void) EXPECT_DECLS; #if defined(HAVE_PKCS7) PKCS7* pkcs7 = NULL; -#ifndef NO_PKCS7_STREAM - int ret; - word32 idx; -#endif WOLFSSL_SMALL_STACK_STATIC byte der[] = { /* outer ContentInfo SEQUENCE (99 bytes content) */ @@ -6402,24 +6466,92 @@ int test_wc_PKCS7_VerifySignedData_DegenerateMinimal(void) pkcs7 = NULL; #ifndef NO_PKCS7_STREAM - /* same bundle fed one byte at a time */ + /* same bundle fed at every chunk size, including one byte at a time: + * chunk boundaries that land mid-octet-string used to leave the + * stream's totalRd tracking out of sync, capping stage 4's expected + * read to less than the signerInfos SET tag/length needs and + * stalling on WC_PKCS7_WANT_READ_E forever. */ + EXPECT_TEST(test_wc_PKCS7_VerifySignedData_ChunkSweep_once(der, derSz, + 1, 60)); +#endif /* !NO_PKCS7_STREAM */ + +#endif /* HAVE_PKCS7 */ + return EXPECT_RESULT(); +} + +/* + * Same shape as test_wc_PKCS7_VerifySignedData_DegenerateMinimal, but with + * an empty certificates [0] SET and empty crls [1] SET both present ahead + * of the empty signerInfos SET ("A0 00 A1 00 31 00" footer). Structurally + * valid, and must verify the same way whether fed in one shot or in small + * chunks that land the footer partway into the stream's internal buffer. + */ +int test_wc_PKCS7_VerifySignedData_DegenerateEmptyCertsCrls(void) +{ + EXPECT_DECLS; +#if defined(HAVE_PKCS7) + PKCS7* pkcs7 = NULL; + + WOLFSSL_SMALL_STACK_STATIC byte der[] = { + /* outer ContentInfo SEQUENCE (103 bytes content) */ + 0x30, 0x67, + /* contentType OID signedData */ + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x02, + /* [0] EXPLICIT (90 bytes content) */ + 0xA0, 0x5A, + /* SignedData SEQUENCE (88 bytes content) */ + 0x30, 0x58, + /* version INTEGER 1 */ + 0x02, 0x01, 0x01, + /* digestAlgorithms SET (empty - degenerate) */ + 0x31, 0x00, + /* encapContentInfo SEQUENCE (75 bytes content) */ + 0x30, 0x4B, + /* eContentType OID 1.2.840.113549.1.7.1 (data) */ + 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x01, + /* eContent [0] EXPLICIT (62 bytes content) */ + 0xA0, 0x3E, + /* OCTET STRING (60 bytes content) */ + 0x04, 0x3C, + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F, + 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, + 0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, 0x1E, 0x1F, + 0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, + 0x28, 0x29, 0x2A, 0x2B, 0x2C, 0x2D, 0x2E, 0x2F, + 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, + 0x38, 0x39, 0x3A, 0x3B, + /* certificates [0] (empty) */ + 0xA0, 0x00, + /* crls [1] (empty) */ + 0xA1, 0x00, + /* signerInfos SET (empty - degenerate end) */ + 0x31, 0x00 + }; + word32 derSz = (word32)sizeof(der); + + /* single-shot call */ ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); - - ret = WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E); - for (idx = 0; idx < derSz && ret != 0; idx++) { - ret = wc_PKCS7_VerifySignedData(pkcs7, der + idx, 1); - if (ret < 0 && ret != WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)) { - break; - } - } - ExpectIntEQ(ret, 0); + ExpectIntEQ(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), 0); if (pkcs7 != NULL) { ExpectIntEQ(pkcs7->contentSz, 60); ExpectNotNull(pkcs7->content); } wc_PKCS7_Free(pkcs7); + pkcs7 = NULL; + +#ifndef NO_PKCS7_STREAM + /* same bundle fed at every chunk size: chunk boundaries that leave the + * "A0 00 A1 00 31 00" footer partly in the stream's internal buffer + * used to make stage 3 undercount the bytes still available (its cap + * dropped the buffered-but-unparsed count instead of adding it in), + * and separately left HandleOctetStrings' totalRd baseline stale + * across a buffered-to-direct read transition, double-counting the + * last content byte and starving stage 6 of the final length byte. */ + EXPECT_TEST(test_wc_PKCS7_VerifySignedData_ChunkSweep_once(der, derSz, + 1, 60)); #endif /* !NO_PKCS7_STREAM */ #endif /* HAVE_PKCS7 */ @@ -6437,6 +6569,7 @@ int test_wc_PKCS7_VerifySignedData_TruncSignerInfosTag(void) EXPECT_DECLS; #if defined(HAVE_PKCS7) PKCS7* pkcs7 = NULL; + int ret; WOLFSSL_SMALL_STACK_STATIC byte der[] = { 0x30, 0x63, @@ -6465,11 +6598,16 @@ int test_wc_PKCS7_VerifySignedData_TruncSignerInfosTag(void) ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); - ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), 0); - ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), - WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)); + ret = wc_PKCS7_VerifySignedData(pkcs7, der, derSz); + ExpectIntNE(ret, 0); + ExpectIntNE(ret, WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)); wc_PKCS7_Free(pkcs7); +#ifndef NO_PKCS7_STREAM + EXPECT_TEST(test_wc_PKCS7_VerifySignedData_ChunkSweep_once(der, derSz, + 1, 0)); +#endif + #endif /* HAVE_PKCS7 */ return EXPECT_RESULT(); } @@ -6485,10 +6623,7 @@ int test_wc_PKCS7_VerifySignedData_NoSignerInfosTag(void) EXPECT_DECLS; #if defined(HAVE_PKCS7) PKCS7* pkcs7 = NULL; -#ifndef NO_PKCS7_STREAM int ret; - word32 idx; -#endif WOLFSSL_SMALL_STACK_STATIC byte der[] = { 0x30, 0x61, @@ -6518,29 +6653,18 @@ int test_wc_PKCS7_VerifySignedData_NoSignerInfosTag(void) ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); - ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), 0); - ExpectIntNE(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), - WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)); + ret = wc_PKCS7_VerifySignedData(pkcs7, der, derSz); + ExpectIntNE(ret, 0); + ExpectIntNE(ret, WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)); wc_PKCS7_Free(pkcs7); pkcs7 = NULL; #ifndef NO_PKCS7_STREAM - /* same bundle fed one byte at a time: must not end stuck on - * WANT_READ_E once all available bytes are consumed */ - ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); - ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); - ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); - - ret = WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E); - for (idx = 0; idx < derSz && ret != 0; idx++) { - ret = wc_PKCS7_VerifySignedData(pkcs7, der + idx, 1); - if (ret < 0 && ret != WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)) { - break; - } - } - ExpectIntNE(ret, 0); - ExpectIntNE(ret, WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)); - wc_PKCS7_Free(pkcs7); + /* same bundle fed at every chunk size, including one byte at a time: + * must not end stuck on WANT_READ_E once all available bytes are + * consumed */ + EXPECT_TEST(test_wc_PKCS7_VerifySignedData_ChunkSweep_once(der, derSz, + 1, 0)); #endif /* !NO_PKCS7_STREAM */ #endif /* HAVE_PKCS7 */ @@ -6605,13 +6729,44 @@ int test_wc_PKCS7_VerifySignedData_DegenerateNonEmptyDigestAlgos(void) ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); wc_PKCS7_AllowDegenerate(pkcs7, 0); ExpectIntEQ(wc_PKCS7_VerifySignedData(pkcs7, der, derSz), - PKCS7_NO_SIGNER_E); + WC_NO_ERR_TRACE(PKCS7_NO_SIGNER_E)); wc_PKCS7_Free(pkcs7); #endif /* HAVE_PKCS7 */ return EXPECT_RESULT(); } +/* + * A genuine, non-degenerate SignedData bundle with a real RSA signerInfos + * entry must still verify successfully when the caller has called + * wc_PKCS7_AllowDegenerate(pkcs7, 0). The degenerate flag computed from + * the signerInfos SET length is also used to reject degenerate bundles + * under that setting, so this confirms a real signer does not get + * misclassified as degenerate and rejected along with them. + */ +int test_wc_PKCS7_VerifySignedData_NoDegenerateAcceptsRealSigner(void) +{ + EXPECT_DECLS; +#if defined(HAVE_PKCS7) && !defined(NO_FILESYSTEM) && !defined(NO_RSA) + PKCS7* pkcs7 = NULL; + byte output[6000]; + word32 outputSz = sizeof(output); + byte data[] = "Test data to encode."; + + ExpectIntGT((outputSz = (word32)CreatePKCS7SignedData(output, + (int)outputSz, data, (word32)sizeof(data), 0, 0, 0, RSA_TYPE)), 0); + + ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId)); + ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0); + ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0); + wc_PKCS7_AllowDegenerate(pkcs7, 0); + ExpectIntEQ(wc_PKCS7_VerifySignedData(pkcs7, output, outputSz), 0); + wc_PKCS7_Free(pkcs7); + +#endif /* HAVE_PKCS7 && !NO_FILESYSTEM && !NO_RSA */ + return EXPECT_RESULT(); +} + #if defined(HAVE_PKCS7) && !defined(NO_RSA) && !defined(NO_SHA256) && \ defined(USE_CERT_BUFFERS_2048) /* diff --git a/tests/api/test_pkcs7.h b/tests/api/test_pkcs7.h index cd0c9a8c1a3..706d987a405 100644 --- a/tests/api/test_pkcs7.h +++ b/tests/api/test_pkcs7.h @@ -79,9 +79,11 @@ int test_wc_PKCS7_VerifySignedData_IndefLenOOB(void); int test_wc_PKCS7_VerifySignedData_TruncEContentTag(void); int test_wc_PKCS7_VerifySignedData_TruncCertSetTag(void); int test_wc_PKCS7_VerifySignedData_DegenerateMinimal(void); +int test_wc_PKCS7_VerifySignedData_DegenerateEmptyCertsCrls(void); int test_wc_PKCS7_VerifySignedData_TruncSignerInfosTag(void); int test_wc_PKCS7_VerifySignedData_NoSignerInfosTag(void); int test_wc_PKCS7_VerifySignedData_DegenerateNonEmptyDigestAlgos(void); +int test_wc_PKCS7_VerifySignedData_NoDegenerateAcceptsRealSigner(void); int test_wc_PKCS7_VerifySignedData_NoDigestParams(void); @@ -138,9 +140,11 @@ int test_wc_PKCS7_VerifySignedData_NoDigestParams(void); TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_TruncEContentTag), \ TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_TruncCertSetTag), \ TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_DegenerateMinimal), \ + TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_DegenerateEmptyCertsCrls), \ TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_TruncSignerInfosTag), \ TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_NoSignerInfosTag), \ TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_DegenerateNonEmptyDigestAlgos), \ + TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_NoDegenerateAcceptsRealSigner), \ TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_NoDigestParams) #define TEST_PKCS7_ENCRYPTED_DATA_DECLS \ diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index bb566cf1aeb..0a5f3f09f4e 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -6662,6 +6662,13 @@ static int wc_PKCS7_HandleOctetStrings(wc_PKCS7* pkcs7, byte* in, word32 inSz, pkcs7->stream->expected, &msg, idx)) != 0) { break; } + /* re-sync totalRd baseline to the fresh idx. Without this, a byte + * that wc_PKCS7_AddDataToStream() already charged to totalRd while + * buffering (growing stream->length) gets charged again below when + * this loop switches to reading directly from "in" starting at + * that same position, since *tmpIdx would still hold a position + * from before the buffered run started. */ + *tmpIdx = *idx; msgSz = (pkcs7->stream->length > 0)? pkcs7->stream->length:inSz; @@ -7517,21 +7524,26 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf, * malformed fails there with a real parse error. */ pkcs7->stream->expected = (ASN_TAG_SZ + MAX_LENGTH_SZ) * 2; - if (!pkcs7->stream->indefLen && - pkcs7->stream->totalRd >= pkcs7->stream->maxLen) { - /* definite-length bundle with no bytes left per the outer - * length: force stage 4's bounds check to fail now instead - * of requesting bytes that will never arrive. For - * indefinite-length (BER) bundles maxLen is only a running - * estimate, so totalRd catching up to it does not mean the - * bundle is actually exhausted. */ - pkcs7->stream->expected = 0; - } - else if (pkcs7->stream->totalRd < pkcs7->stream->maxLen && - pkcs7->stream->expected > - pkcs7->stream->maxLen - pkcs7->stream->totalRd) { - pkcs7->stream->expected = - pkcs7->stream->maxLen - pkcs7->stream->totalRd; + if (!pkcs7->stream->indefLen) { + /* cap to what can actually still show up: bytes already + * buffered (stream->length) plus whatever the outer + * length still allows past totalRd. Matches the same + * (maxLen - totalRd) + length expression used at every + * other stage in this function. For indefinite-length + * (BER) bundles maxLen is only a running estimate, so + * this cap does not apply. */ + if (pkcs7->stream->totalRd >= pkcs7->stream->maxLen) { + if (pkcs7->stream->expected > pkcs7->stream->length) { + pkcs7->stream->expected = pkcs7->stream->length; + } + } + else if (pkcs7->stream->expected > + (pkcs7->stream->maxLen - pkcs7->stream->totalRd) + + pkcs7->stream->length) { + pkcs7->stream->expected = + (pkcs7->stream->maxLen - pkcs7->stream->totalRd) + + pkcs7->stream->length; + } } #else @@ -7673,6 +7685,11 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf, } maxIdx = idx + pkcs7->stream->expected; + + /* re-sync totalRd baseline to the fresh idx, same as STAGE6 + * does; otherwise it can still hold a stale position left + * over from stage 3's internal stream buffer. */ + stateIdx = idx; #endif /* !NO_PKCS7_STREAM */ if (pkiMsg2 == NULL || pkiMsg2Sz == 0) { @@ -7782,6 +7799,11 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf, pkiMsg2Sz = (pkcs7->stream->length > 0)? pkcs7->stream->length: srcSz; + /* re-sync totalRd baseline to the fresh idx, same as STAGE4 + * and STAGE6 do; otherwise it can still hold a stale position + * left over from STAGE4's internal stream buffer. */ + stateIdx = idx; + wc_PKCS7_StreamGetVar(pkcs7, &pkiMsg2Sz, 0, &length); /* restore content */ @@ -8086,6 +8108,30 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf, NO_USER_CHECK) < 0) ret = ASN_PARSE_E; + #ifndef NO_PKCS7_STREAM + /* claimed signerInfos content must fit in what can still + * arrive, or stage 7 stalls on WANT_READ_E forever */ + if (ret == 0 && !pkcs7->stream->indefLen) { + word32 avail; + if (pkcs7->stream->length > 0) { + avail = pkcs7->stream->length - idx; + if (pkcs7->stream->totalRd < pkcs7->stream->maxLen) { + avail += pkcs7->stream->maxLen - + pkcs7->stream->totalRd; + } + } + else { + word32 used = pkcs7->stream->totalRd + + (idx - stateIdx); + avail = (used < pkcs7->stream->maxLen) ? + pkcs7->stream->maxLen - used : 0; + } + if ((word32)length > avail) { + ret = ASN_PARSE_E; + } + } + #endif + /* Update degenerate flag based on if signerInfos SET is empty. * The earlier degenerate check at digestAlgorithms is an early * optimization, but depending on degenerate case may not be @@ -8129,18 +8175,6 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf, } else { pkcs7->stream->expected = (word32)length; - if (pkcs7->stream->totalRd >= pkcs7->stream->maxLen) { - /* definite-length bundle with no bytes left per the - * outer length: force stage 7's bounds check to fail - * now instead of requesting bytes that will never - * arrive. Do NOT cap expected down further than that - * when bytes may still remain -- expected here is the - * full signerInfo content size that stage 7 needs - * buffered at once, not just header lookahead, so - * shrinking it while genuinely more data is still due - * from a future call would corrupt the parse window. */ - pkcs7->stream->expected = 0; - } } wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_VERIFY_STAGE7);