Skip to content

Zero EncryptedInfo in ProcessChainBufferCRL and reset info->set on parse - #11046

Open
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_7352
Open

Zero EncryptedInfo in ProcessChainBufferCRL and reset info->set on parse#11046
yosuke-wolfssl wants to merge 1 commit into
wolfSSL:masterfrom
yosuke-wolfssl:fix/f_7352

Conversation

@yosuke-wolfssl

Copy link
Copy Markdown
Contributor

Problem

ProcessChainBufferCRL() declared a stack EncryptedInfo and passed it to PemToDer() without zeroing it — unlike every other caller in the tree. A PEM CRL carries no Proc-Type: header, so wc_EncryptedInfoParse() returns without ever writing info->set, leaving PemToDer() to read stack residue. When that residue is non-zero:

  • encrypted_key is set spuriously.
  • The CRL header does not match the non-encrypted early return, so control falls into the encrypted-key block.
  • info->passwd_cb is then invoked through the uninitialized pointer, with an uninitialized argument, on a path that should never touch password callbacks.

Reachable from the public wolfSSL_CTX_load_verify_buffer() with WOLFSSL_FILETYPE_PEM. --enable-wpas alone enables all three required macros (WOLFSSL_WPAS, HAVE_CRL, WOLFSSL_ENCRYPTED_KEYS) — the configuration used to link wpa_supplicant/hostapd.

Fix (src/ssl_load.c)

     WOLFSSL_MSG("Trying a CRL");
+    XMEMSET(&info, 0, sizeof(info));
     ret = PemToDer(buff, sz, CRL_TYPE, &der, NULL, &info, NULL);

Defense in depth in wolfcrypt/src/asn.c: wc_EncryptedInfoParse() now clears info->set on entry, so no future caller can be bitten by a stale flag. set has exactly two references tree-wide, so nothing relied on it persisting across a parse.

Closes f-7352.

Tests (tests/api.c)

  • test_wolfSSL_CTX_load_verify_buffer_pem_crl — first coverage of the CRL-in-chain fall-through. Loads ca-cert.pem + crl/crl.pem as one PEM buffer and asserts the CRL actually reached the cert manager.
  • A case in test_wc_PemToDer passing a deliberately dirtied info.set = 1 with CRL_TYPE. This is the deterministic regression guard: it needs no sanitizer and runs in ordinary builds. Appended last and re-zeroed so it cannot contaminate the existing assertions.

Verification

Build Result
--enable-all 1770 passed, 0 failed
--enable-wpas --enable-opensslextra 1106 passed, 0 failed
make check, both configs 0 failures

Negative controls: reverting the asn.c reset makes the new wc_PemToDer case fail with NO_PASSWORD (-176) and nothing else. Forcing worst-case residue with both fixes reverted aborts the suite with SIGBUS on the indirect call, confirming the reported impact. ASan + UBSan clean; no warnings under -Werror.

@yosuke-wolfssl yosuke-wolfssl self-assigned this Aug 4, 2026
Copilot AI lite review requested due to automatic review settings August 4, 2026 05:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Ready to approve

The changes are narrowly scoped, directly address an unsafe uninitialized-state path, and include targeted regression tests that exercise the affected code paths.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Fixes a reachable uninitialized-read / uninitialized-callback path when parsing PEM CRLs in certificate chains by ensuring EncryptedInfo state is deterministically initialized and cannot retain stale “encrypted” flags across parses.

Changes:

  • Zero-initialize EncryptedInfo in ProcessChainBufferCRL() before calling PemToDer().
  • Reset info->set on entry to wc_EncryptedInfoParse() as defense-in-depth against stale state.
  • Add regression tests covering the CRL-in-chain fall-through and a deterministic wc_PemToDer() CRL case with a dirtied info.set.
File summaries
File Description
wolfcrypt/src/asn.c Ensures EncryptedInfo parse always starts with set = 0, preventing stale “encrypted” state from influencing PemToDer() behavior.
src/ssl_load.c Zeroes stack EncryptedInfo in the CRL chain parsing path to avoid reading stack residue.
tests/api.c Adds coverage for the CRL fall-through path and a targeted regression test for stale info.set with CRL_TYPE.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #11046

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

No new issues found in the changed files. ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants