Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds defensive argument validation and strengthens cleanup/zeroization of sensitive buffers across cryptography and TLS 1.3 code paths.
Changes:
- Add NULL argument checks in DES CBC encrypt/decrypt and crypt key derivation helpers.
- Harden HPKE context open API by validating more input pointers.
- Improve TLS 1.3 secret-handling by consolidating error paths and zeroizing intermediate buffers (binder keys, MACs, HMAC temp).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| wolfcrypt/src/wc_encrypt.c | Adds early BAD_FUNC_ARG returns for NULL pointers in DES CBC and CryptKey helpers. |
| wolfcrypt/src/hpke.c | Extends argument validation in wc_HpkeContextOpenBase. |
| src/tls13.c | Refactors multiple early returns to cleanup: for consistent ForceZero() of sensitive buffers. |
| src/internal.c | Simplifies preprocessor guards around “encrypted data too long” logic and restructures TLS1.3 vs non-TLS1.3 branches. |
Comments suppressed due to low confidence (1)
src/internal.c:1
- This change removes the previous compilation guards around the
tooLong/record_overflow check (previously only compiled whenWOLFSSL_TLS13orWOLFSSL_EXTRA_ALERTSwas enabled). As written, the non-TLS1.3MAX_TLS_CIPHER_SZcheck will now compile and run even whenWOLFSSL_EXTRA_ALERTSis not defined, which is a functional change and can also break builds ifMAX_TLS_CIPHER_SZ(or related alert behavior) is not available under those configurations. Consider restoring an equivalent#if defined(WOLFSSL_TLS13) || defined(WOLFSSL_EXTRA_ALERTS)guard around thetooLonglogic (or otherwise ensuring the macro/behavior is valid in all builds) to avoid changing behavior across feature-flag combinations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Explicitly call the ANSI version of the InetPton function to avoid an incorrect cast to PCWSTR when the input string is a standard character pointer.
Member
Author
|
Retest this please Error cloning |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.