Skip to content

Fix truncation and noDegenerate bypasses in wc_PKCS7_VerifySignedData #11017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
miyazakh wants to merge 5 commits into
base: master
Choose a base branch
from
+313 −29
Open

Fix truncation and noDegenerate bypasses in wc_PKCS7_VerifySignedData #11017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fcb13ed
fix f7302 pkcs7
miyazakh Jul 30, 2026
72c4aa5
additional fix
miyazakh Jul 30, 2026
a600695
tests: remove bare scope block in PKCS7 DegenerateMinimal stream test
miyazakh Jul 31, 2026
8777c2b
pkcs7: fix VerifySignedData stall when signerInfos is missing entirely
miyazakh Jul 31, 2026
59c4c70
pkcs7: don't zero-cap stage 3 expected for indefinite-length bundles
miyazakh Jul 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
289 changes: 279 additions & 10 deletions tests/api/test_pkcs7.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = {
Expand Down Expand Up @@ -6337,7 +6333,280 @@ 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;
#ifndef NO_PKCS7_STREAM
int ret;
word32 idx;
#endif

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 */
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);

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 [Medium] TruncSignerInfosTag test does not catch WC_PKCS7_WANT_READ_E stall · Weak or missing assertions

The 31 01 signerInfos header (declared length 1, 0 bytes available) makes the default streaming build's single-shot call return WC_PKCS7_WANT_READ_E (a stall, not a rejection), yet ExpectIntNE(ret, 0) passes since -270 != 0.

Fix: Add ExpectIntNE(ret, WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)) as done in the sibling NoSignerInfosTag test.

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;
#ifndef NO_PKCS7_STREAM
int ret;
word32 idx;
#endif

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);

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

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

Expand Down
8 changes: 8 additions & 0 deletions tests/api/test_pkcs7.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ 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_DegenerateNonEmptyDigestAlgos(void);
int test_wc_PKCS7_VerifySignedData_NoDigestParams(void);


Expand Down Expand Up @@ -133,6 +137,10 @@ 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_DegenerateNonEmptyDigestAlgos), \
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_NoDigestParams)

#define TEST_PKCS7_ENCRYPTED_DATA_DECLS \
Expand Down
45 changes: 26 additions & 19 deletions wolfcrypt/src/pkcs7.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -7506,26 +7506,33 @@ 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->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;
}

#else
/* Break out before content because it can be optional in degenerate
Expand Down
Loading