fix: ChaCha20-Poly1305 and XChaCha20-Poly1305 one-shot APIs allow empty plaintext#10252
fix: ChaCha20-Poly1305 and XChaCha20-Poly1305 one-shot APIs allow empty plaintext#10252MarkAtwood wants to merge 1 commit intowolfSSL:masterfrom
Conversation
…mpty plaintext Three functions rejected zero-length plaintext with BAD_FUNC_ARG: 1. wc_ChaCha20Poly1305_Decrypt: required outPlaintext != NULL even when inCiphertextLen is 0. RFC 8439 Section 2.8 permits empty plaintext (authentication-only mode). 2. wc_ChaCha20Poly1305_UpdateData: required inData/outData != NULL unconditionally, and called wc_Chacha_Process with NULL pointers even when dataLen is 0 (which also rejects NULL). 3. wc_XChaCha20Poly1305_crypt_oneshot: required dst != NULL even when dst_len is 0. Fix: gate all NULL checks and processing calls on the respective length being > 0. Also guard the ForceZero error cleanup in the one-shot decrypt path, which would pass NULL to ForceZero on error with zero-length plaintext. Note: the streaming API Final() fix for empty plaintext was merged upstream separately (PR wolfSSL#10040). This commit fixes the one-shot and UpdateData paths that the streaming fix did not cover. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Found via Wycheproof test vectors.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates ChaCha20-Poly1305 / XChaCha20-Poly1305 one-shot and update APIs to correctly support zero-length plaintext inputs (authentication-only) without returning BAD_FUNC_ARG, addressing failures found by Wycheproof vectors.
Changes:
- Relaxed argument validation to allow
outPlaintext/dstto beNULLwhen the corresponding plaintext length is0. - Prevented NULL-pointer use in the error cleanup path by guarding the
ForceZero()call when plaintext length is0. - Avoided calling
wc_Chacha_Process()with NULL pointers for zero-lengthUpdateData()operations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| if ((dst == NULL) || (src == NULL)) { | ||
| if ((dst_len > 0 && dst == NULL) || (src == NULL)) { |
There was a problem hiding this comment.
wc_XChaCha20Poly1305_crypt_oneshot() now allows dst == NULL for dst_len == 0, but still rejects src == NULL unconditionally. For consistency with the other APIs changed in this PR (which permit NULL input pointers when the corresponding length is 0), consider changing this to only require src != NULL when src_len > 0 (or when the function will actually read from src). This would make encrypting an empty plaintext (where src_len == 0) work even if callers pass src == NULL (common C convention for zero-length buffers).
| if ((dst_len > 0 && dst == NULL) || (src == NULL)) { | |
| if ((dst_len > 0 && dst == NULL) || (src_len > 0 && src == NULL)) { |
Summary
Three functions rejected zero-length plaintext with
BAD_FUNC_ARG:wc_ChaCha20Poly1305_Decrypt— requiredoutPlaintext != NULLeven wheninCiphertextLenis 0. RFC 8439 §2.8 permits empty plaintext (authentication-only mode).wc_ChaCha20Poly1305_UpdateData— requiredinData/outData != NULLunconditionally, and calledwc_Chacha_Processwith NULL whendataLenis 0.wc_XChaCha20Poly1305_crypt_oneshot— requireddst != NULLeven whendst_lenis 0.Also guards the
ForceZeroerror-cleanup path in the one-shot decrypt, which would pass NULL toForceZeroon error with zero-length plaintext.Note: the streaming Final() fix was a separate commit; this covers the one-shot and UpdateData paths.
Found via Wycheproof test vectors.
Test plan
/cc @wolfSSL-Fenrir-bot please review