fix: AES-EAX allow empty plaintext#10251
Conversation
The one-shot wc_AesEaxEncryptAuth/wc_AesEaxDecryptAuth and the incremental wc_AesEaxEncryptUpdate/wc_AesEaxDecryptUpdate functions unconditionally required non-NULL out/in/authIn pointers, returning BAD_FUNC_ARG even when the corresponding length was 0. EAX mode with empty plaintext is a valid authentication-only operation that produces just an authentication tag (OMAC over nonce and AAD). This is permitted by the EAX specification (Bellare, Rogaway, Wagner 2004) and exercised by Wycheproof test vectors. Fix: gate NULL pointer checks on the corresponding length being > 0. In the incremental Update functions, skip the AES-CTR and CMAC ciphertext update when inSz is 0 to avoid passing NULL to wc_AesCtrEncrypt (which also rejects NULL unconditionally). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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 AES-EAX one-shot and incremental APIs to support authentication-only operations by allowing NULL plaintext/ciphertext pointers when the corresponding length is 0 (as permitted by the EAX spec and required by Wycheproof vectors).
Changes:
- Relax NULL pointer checks in
wc_AesEaxEncryptAuth/wc_AesEaxDecryptAuthwheninSz == 0and/orauthInSz == 0. - In
wc_AesEaxEncryptUpdate/wc_AesEaxDecryptUpdate, skip CTR and ciphertext-CMAC updates wheninSz == 0to avoid callingwc_AesCtrEncryptwith NULL pointers.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| int ret; | ||
|
|
||
| if (eax == NULL || out == NULL || in == NULL) { | ||
| if (eax == NULL || (inSz > 0 && (out == NULL || in == NULL))) { |
There was a problem hiding this comment.
wc_AesEaxEncryptUpdate still doesn't validate authIn when authInSz > 0. If the function later CMAC-updates AAD when authInSz is non-zero, passing authIn == NULL will result in a NULL dereference. Fix by extending the argument validation to also require authIn != NULL when authInSz > 0 (mirroring the one-shot Auth functions).
| if (eax == NULL || (inSz > 0 && (out == NULL || in == NULL))) { | |
| if (eax == NULL || (inSz > 0 && (out == NULL || in == NULL)) || | |
| (authInSz > 0 && authIn == NULL)) { |
| int ret; | ||
|
|
||
| if (eax == NULL || out == NULL || in == NULL) { | ||
| if (eax == NULL || (inSz > 0 && (out == NULL || in == NULL))) { |
There was a problem hiding this comment.
wc_AesEaxDecryptUpdate has the same gap as EncryptUpdate: there is no guard ensuring authIn != NULL when authInSz > 0. If the function proceeds to CMAC-update AAD for non-zero authInSz, this can crash. Add a length-gated NULL check for authIn in the argument validation.
| if (eax == NULL || (inSz > 0 && (out == NULL || in == NULL))) { | |
| if (eax == NULL || (inSz > 0 && (out == NULL || in == NULL)) || | |
| (authInSz > 0 && authIn == NULL)) { |
|
Summary
wc_AesEaxEncryptAuth,wc_AesEaxDecryptAuth,wc_AesEaxEncryptUpdate, andwc_AesEaxDecryptUpdateunconditionally required non-NULLout/in/authInpointers, returningBAD_FUNC_ARGeven when the corresponding length was 0.EAX mode with empty plaintext is a valid authentication-only operation (OMAC over nonce and AAD only), permitted by the EAX specification (Bellare, Rogaway, Wagner 2004) and exercised by Wycheproof test vectors.
Fix: gate NULL pointer checks on the corresponding length being > 0. In the incremental Update functions, skip the AES-CTR and CMAC ciphertext update when
inSzis 0 to avoid passing NULL towc_AesCtrEncrypt.Found via Wycheproof test vectors.
Test plan
/cc @wolfSSL-Fenrir-bot please review