Harden PKCS#7 FlattenEncodedAttribs

[embhorn]

Description

Use WC_SAFE_SUM_WORD32 with count values in FlattenEncodedAttribs and EncodeAttributes

Fixes zd21942

Testing

Added three regression tests

Checklist

• added tests
• updated/added doxygen
• updated appropriate READMEs
• Updated manual and documentation

[Copilot]

Pull request overview

This PR hardens the PKCS#7 attribute encoding path against word32 size accumulation overflow (notably in EncodeAttributes() and FlattenEncodedAttribs()), and adds regression tests to ensure oversized application-supplied attribute lengths are rejected with an error instead of causing undersized allocations and subsequent buffer overflows.

Changes:

• Add WC_SAFE_SUM_WORD32 overflow checks when computing per-attribute flattened DER sizes and guard the running attribute size total.
• Update multiple call sites to treat EncodeAttributes() negative returns as errors and perform appropriate cleanup.
• Add three regression tests covering overflow scenarios in SignedData, AuthEnvelopedData, and EncryptedData encode paths.

Reviewed changes

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

File	Description
wolfcrypt/src/pkcs7.c	Adds overflow-checked size arithmetic for PKCS#7 attribute encoding/flattening and propagates errors safely to callers.
tests/api/test_pkcs7.h	Declares new regression test entry points and registers them in the PKCS#7 test groups.
tests/api/test_pkcs7.c	Implements regression tests that exercise the new overflow guards across multiple PKCS#7 encode paths.

---

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

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #10636

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

⚠️ Review incomplete — one or more scan targets failed before findings could be produced. See the Fenrir PR review detail page for logs.

[github-actions]

MemBrowse Memory Report

gcc-arm-cortex-m4-pkcs7

• FLASH: .text +128 B (+0.1%, 210,801 B / 262,144 B, total: 80% used)
  No memory changes detected for:
• gcc-arm-cortex-m0plus
• gcc-arm-cortex-m3
• gcc-arm-cortex-m4
• 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-openssl-compat
• gcc-arm-cortex-m4-pq
• gcc-arm-cortex-m4-rsa-only
• gcc-arm-cortex-m4-sp-math
• gcc-arm-cortex-m4-tls12
• gcc-arm-cortex-m4-tls13
• gcc-arm-cortex-m7
• gcc-arm-cortex-m7-pq
• gcc-arm-cortex-m7-tls13
• linuxkm-pie
• linuxkm-standard
• stm32-sim-stm32h753

[dgarske]

Skoll Code Review

Scan type: reviewOverall recommendation: COMMENT
Findings: 2 total — 2 posted, 0 skipped
2 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [Low] New int-range overflow path and FlattenEncodedAttribs guard not exercised by tests — wolfcrypt/src/pkcs7.c:1677-1681
• [Info] Redundant pkcs7-heap assignment in EncryptedData overflow test — tests/api/test_pkcs7.c

Review generated by Skoll

[dgarske]

🔵 [Low] New int-range overflow path and FlattenEncodedAttribs guard not exercised by tests

The three new regression tests all use attrib.valueSz = 0xFFFFFFF4U. That value is caught by the first guard (the boundSz WC_SAFE_SUM_WORD32 chain at lines 1656-1661, which overflows word32 on the MAX_SEQ_SZ addition), so it returns BUFFER_E before control ever reaches the second guard added at lines 1677-1681 (the allAttribsSz/WC_MAX_SINT_OF(int) int-range check) or the defense-in-depth guard added in FlattenEncodedAttribs (lines 1791-1796). The int-range path is reachable with a valueSz near INT_MAX (e.g. 0x7FFFFFF0): boundSz stays within word32 range, but attribSz then exceeds WC_MAX_SINT_OF(int) and the new check fires. That distinct error path currently has no coverage.

Fix: Consider adding a test vector with valueSz close to INT_MAX to exercise the second overflow guard; not blocking since the resulting behavior (BUFFER_E) is identical and the check is defensive.

[dgarske]

⚪ [Info] Redundant pkcs7-heap assignment in EncryptedData overflow test

wc_PKCS7_Init(pkcs7, HEAP_HINT, testDevId) already sets pkcs7->heap = HEAP_HINT, so the explicit pkcs7->heap = HEAP_HINT; inside the field-initialization block is redundant. The sibling SignedData/AuthEnveloped tests do not set it. Harmless, just inconsistent.

Fix: Remove the redundant assignment (optional cleanup).