Fix truncation and noDegenerate bypasses in wc_PKCS7_VerifySignedData by miyazakh · Pull Request #11017 · wolfSSL/wolfssl

miyazakh · 2026-07-30T08:39:39Z

Summary

wc_PKCS7_VerifySignedData() had two bugs that let signature verification be silently bypassed (F-7302). Both returned 0 (success) with pkcs7->content populated from unauthenticated data.

Default streaming build (NO_PKCS7_STREAM undefined): a bundle truncated 0-5 bytes short of a complete signerInfos SET was accepted as a valid degenerate (certs-only) end.(f7302)
Both build modes: a well-formed bundle with non-empty digestAlgorithms and empty signerInfos bypassed wc_PKCS7_AllowDegenerate(pkcs7, 0) entirely.

Bug 1: truncation accepted as a valid degenerate end

WC_PKCS7_VERIFY_STAGE3 treated any input with fewer than 6 bytes remaining after the content as a finished, successful parse, unable to distinguish a genuine minimal degenerate end (31 00) from a truncated bundle. Dates back to 3cad0bfe5, which left the gap with an acknowledging comment.

Fix: stage 3 no longer returns early. It caps pkcs7->stream->expected to the bytes actually remaining, then lets stage 4/5/6's existing bounds checks run normally, so truncated input fails there instead of being silently accepted.

Bug 2: noDegenerate bypass via non-empty digestAlgorithms

wc_PKCS7_ParseSignerInfo() only enforced noDegenerate when the whole remaining buffer was empty, never when the accurately-computed degenerate flag (from the real signerInfos SET) was true. No truncation needed to trigger this.

Fix: one line, broadened the guard to also check degenerate == 1:

if (pkcs7->noDegenerate == 1 && (inSz == 0 || degenerate == 1)) {

Tests added

test_wc_PKCS7_VerifySignedData_DegenerateMinimal -- minimal legit degenerate bundle still succeeds.
test_wc_PKCS7_VerifySignedData_TruncSignerInfosTag / NoSignerInfosTag -- truncated/missing signerInfos rejected.
test_wc_PKCS7_VerifySignedData_DegenerateNonEmptyDigestAlgos -- bug 2 case returns PKCS7_NO_SIGNER_E.
Dropped the NO_PKCS7_STREAM-only gate on test_wc_PKCS7_VerifySignedData_TruncCertSetTag; it now passes in streaming mode too.

Testing

Verified in isolated git worktree builds (--enable-pkcs7 --enable-opensslextra, plus CFLAGS=-DNO_PKCS7_STREAM for the non-streaming build), since the primary tree doesn't enable PKCS7. Each bug: negative test failed pre-fix, passed post-fix. No regressions in pkcs7_sd, wolfcrypt/test/testwolfcrypt, or the full tests/unit.test suite, in both build modes.

Copilot

Pull request overview

This pull request fixes two PKCS#7 SignedData verification bypasses in wc_PKCS7_VerifySignedData() that could otherwise return success and populate pkcs7->content from unauthenticated data, and adds regression tests covering both bypass scenarios (streaming and non-streaming builds).

Changes:

Enforce noDegenerate when signerInfos is truly degenerate (empty) even if digestAlgorithms is non-empty.
Remove the streaming stage-3 “< 6 bytes remaining” success early-exit by bounding lookahead to the bytes actually remaining.
Add new API tests for minimal valid degenerate bundles and for truncated/missing signerInfos cases; broaden an existing truncation test to run in both build modes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
wolfcrypt/src/pkcs7.c	Tightens degenerate/noDegenerate enforcement and adjusts streaming footer lookahead to prevent truncation being treated as successful completion.
tests/api/test_pkcs7.h	Adds declarations/registration for new SignedData verification regression tests.
tests/api/test_pkcs7.c	Adds new regression tests for minimal degenerate success, truncation rejection, and non-empty digestAlgorithms + empty signerInfos rejection; updates an existing truncation test gating.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

miyazakh · 2026-07-31T09:10:28Z

            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;
+            }


Confirmed. Reproduced with NoSignerInfosTag (bundle truncated exactly at the outer SignedData length, no signerInfos at all): both single-shot and streaming calls returned WC_PKCS7_WANT_READ_E (-270) instead of a deterministic error, and streaming consumed all available bytes without resolving.

Fixed by capping pkcs7->stream->expected to 0 when totalRd >= maxLen, instead of skipping the cap in that case, so stage 4's existing idx >= pkiMsg2Sz bounds check now fires immediately and returns BUFFER_E.

Also strengthened test_wc_PKCS7_VerifySignedData_NoSignerInfosTag to assert the return code is not WC_PKCS7_WANT_READ_E (the prior ExpectIntNE(ret, 0) check passed even on the stall), and added a streaming byte-at-a-time variant of the same case.

Fixed in 8777c2b.

github-actions · 2026-07-30T08:51:36Z

MemBrowse Memory Report

gcc-arm-cortex-m4

FLASH: .text +64 B (+0.0%, 200,917 B / 262,144 B, total: 77% used)

gcc-arm-cortex-m4-openssl-compat

FLASH: .text +384 B (+0.0%, 771,412 B / 1,048,576 B, total: 74% used)

gcc-arm-cortex-m4-pkcs7

FLASH: .text +64 B (+0.0%, 213,908 B / 262,144 B, total: 82% used)

gcc-arm-cortex-m4-rsa-only

FLASH: .text -64 B (-0.0%, 326,104 B / 1,048,576 B, total: 31% used)

gcc-arm-cortex-m4-tls13

FLASH: .text -64 B (-0.0%, 236,615 B / 262,144 B, total: 90% used)

gcc-arm-cortex-m7-pq

FLASH: .text -64 B (-0.0%, 280,512 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m7-tls13

FLASH: .text -64 B (-0.0%, 236,679 B / 262,144 B, total: 90% used)

linuxkm-pie

Data: __patchable_function_entries +64 B (+0.2%, 26,320 B)

linuxkm-standard

Data: __patchable_function_entries +32 B (+0.1%, 49,008 B)
No memory changes detected for:
gcc-arm-cortex-m0plus
gcc-arm-cortex-m3
gcc-arm-cortex-m4-baremetal
gcc-arm-cortex-m4-crypto-only
gcc-arm-cortex-m4-dtls13
gcc-arm-cortex-m4-min-ecc
gcc-arm-cortex-m4-pq
gcc-arm-cortex-m4-sp-math
gcc-arm-cortex-m4-tls12
gcc-arm-cortex-m7
stm32-sim-stm32h753

Hoist ret/idx declarations to the top of the HAVE_PKCS7 block instead of a standalone { ... } scope, per wolfSSL C coding style.

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.

The totalRd >= maxLen zero-cap added in 8777c2b 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.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #11017

Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

wolfSSL-Fenrir-bot · 2026-07-31T21:59:50Z

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


🟠 [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.

miyazakh added 2 commits July 30, 2026 12:31

fix f7302 pkcs7

fcb13ed

additional fix

72c4aa5

Copilot AI review requested due to automatic review settings July 30, 2026 08:39

Copilot started reviewing on behalf of miyazakh July 30, 2026 08:40 View session

Copilot AI reviewed Jul 30, 2026

View reviewed changes

miyazakh self-assigned this Jul 30, 2026

miyazakh added 3 commits July 31, 2026 17:59

tests: remove bare scope block in PKCS7 DegenerateMinimal stream test

a600695

Hoist ret/idx declarations to the top of the HAVE_PKCS7 block instead of a standalone { ... } scope, per wolfSSL C coding style.

miyazakh requested a review from wolfSSL-Fenrir-bot July 31, 2026 21:40

wolfSSL-Fenrir-bot reviewed Jul 31, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix truncation and noDegenerate bypasses in wc_PKCS7_VerifySignedData - #11017

Fix truncation and noDegenerate bypasses in wc_PKCS7_VerifySignedData#11017
miyazakh wants to merge 5 commits into
wolfSSL:masterfrom
miyazakh:f7302_pkcs7_verify

miyazakh commented Jul 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

miyazakh Jul 31, 2026

Uh oh!

github-actions Bot commented Jul 30, 2026 •

edited

Loading

Uh oh!

wolfSSL-Fenrir-bot left a comment

Uh oh!

wolfSSL-Fenrir-bot Jul 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

miyazakh commented Jul 30, 2026

Summary

Bug 1: truncation accepted as a valid degenerate end

Bug 2: noDegenerate bypass via non-empty digestAlgorithms

Tests added

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

MemBrowse Memory Report

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #11017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jul 30, 2026 •

edited

Loading