Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes an out-of-bounds read risk in PKCS#12 parsing when processing ENCRYPTED_DATA by ensuring subsequent ASN.1 parsing uses the decrypted payload size (not the original ContentInfo size), and adds a regression test case.
Changes:
- Track decrypted content bounds via a new
contentSzvalue and use it for subsequent ASN.1 parsing operations. - Preserve existing behavior for the
DATAContentInfo branch by continuing to useci->dataSz. - Add a new API test covering the encrypted-content parsing path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| wolfcrypt/src/pkcs12.c | Introduces contentSz and switches ASN.1 parsing bounds to ensure decrypted content is parsed within correct limits. |
| tests/api/test_pkcs12.h | Registers the new PKCS#12 encrypted-content bounds test in the API test list. |
| tests/api/test_pkcs12.c | Adds a new test that creates, serializes, re-parses, and parses a PKCS#12 with encrypted content. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Jenkins retest this please |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10172
Scan targets checked: wolfcrypt-api_misuse, wolfcrypt-bugs, wolfcrypt-compliance, wolfcrypt-concurrency, wolfcrypt-consttime, wolfcrypt-defaults, wolfcrypt-mutation, wolfcrypt-portability, wolfcrypt-proptest, wolfcrypt-src, wolfcrypt-zeroize
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.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10172
Scan targets checked: wolfcrypt-api_misuse, wolfcrypt-bugs, wolfcrypt-compliance, wolfcrypt-concurrency, wolfcrypt-consttime, wolfcrypt-defaults, wolfcrypt-mutation, wolfcrypt-portability, wolfcrypt-proptest, wolfcrypt-src, wolfcrypt-zeroize
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
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10172
Scan targets checked: wolfcrypt-api_misuse, wolfcrypt-bugs, wolfcrypt-compliance, wolfcrypt-concurrency, wolfcrypt-consttime, wolfcrypt-defaults, wolfcrypt-mutation, wolfcrypt-portability, wolfcrypt-proptest, wolfcrypt-src, wolfcrypt-zeroize
No new issues found in the changed files. ✅
Description
When an ENCRYPTED_DATA ContentInfo is decrypted, the code allocates a heap buffer, calls DecryptContent() to decrypt in-place, then reassigns the data pointer to the decrypted buffer. However, all subsequent ASN.1 parsing operations continue using ci->dataSz (the original, larger ContentInfo size) as the bounds parameter, instead of the actual decrypted content size returned by DecryptContent().
Fixes zd21568
Testing
Added
test_wc_PKCS12_encrypted_content_boundsChecklist