Security hardening, input validation, memory-safety, and key zeroization fixes#438
Open
aidangarske wants to merge 34 commits into
Open
Security hardening, input validation, memory-safety, and key zeroization fixes#438aidangarske wants to merge 34 commits into
aidangarske wants to merge 34 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a batch of Fenrir “Medium” security findings in wolfProvider by hardening crypto/provider code paths against out-of-bounds reads, integer truncation/overflow, unsafe parameter handling, and missing validation; it also adds targeted negative unit tests to exercise the fixed failure modes.
Changes:
- Fix multiple memory-safety issues in TLS record decryption, key import, and PEM/DER decode paths (OOB reads, stale AAD, bad-padding handling).
- Add input-validation and integer-bound checks across KDF/MAC/key management APIs (iteration bounds, diversifier/id ranges, truncation/overflow guards, explicit DH/ECC validation).
- Improve robustness/operational behavior (libctx child creation dispatch, post-fork mutex reinit) and add zeroization/clear-free for sensitive state.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit.h | Registers new negative/security regression tests in the unit test interface. |
| test/unit.c | Adds the new tests to the unit test runner list. |
| test/test_tls_cbc.c | Adds a split-record TLS CBC decrypt test to exercise the buffered-block TLS path. |
| test/test_rsa.c | Adds a regression test to verify RSA-PSS salt length DER encoding for values ≥ 256. |
| test/test_pbkdf2.c | Adds tests for PBKDF2 iteration truncation and PKCS12KDF diversifier id validation. |
| test/test_hmac.c | Adds a test ensuring HMAC init without a digest fails safely. |
| test/test_hkdf.c | Adds a test to validate HKDF ctx duplication behavior/state (or unsupported dup). |
| test/test_ecx.c | Adds a test rejecting zero-length X25519 private key import (no OOB read). |
| test/test_ecc.c | Adds tests for ECDSA digest-set atomicity, truncated PEM decode rejection, EC scalar and group-name import validation. |
| test/test_dh.c | Adds tests for explicit DH param_check validation and non-NUL-terminated group-name import safety. |
| src/wp_wolfprov.c | Fixes child libctx creation by preserving the original core dispatch table pointer. |
| src/wp_rsa_kmgmt.c | Caps passphrase-callback length and fixes RSA-PSS salt length DER encoding (2-byte encoding). |
| src/wp_pbkdf2.c | Rejects PBKDF2 iterations above INT_MAX and validates PKCS12KDF diversifier id range. |
| src/wp_params.c | Tightens UTF8 string size checks when copying from OSSL_PARAM. |
| src/wp_kdf_exch.c | Changes KDF key-exchange ctx duplication to report unsupported rather than duplicating incorrectly. |
| src/wp_internal.c | Adds post-fork mutex reinit, strengthens password-length handling, adds zeroization, and guards DER BIO read accumulation overflow. |
| src/wp_hmac.c | Clears HMAC context on free and rejects key-set attempts when no digest is configured. |
| src/wp_ecx_kmgmt.c | Rejects zero-length private key material during ECX import to prevent OOB reads. |
| src/wp_ecdsa_sig.c | Makes digest selection atomic: only commit digest state after validation/init succeeds. |
| src/wp_ecc_kmgmt.c | Bounds/copies non-NUL-terminated group-name params and validates imported private scalar range. |
| src/wp_dh_kmgmt.c | Implements explicit DH domain-parameter validation and bounds/copies non-NUL-terminated group-name params. |
| src/wp_des.c | Adds a TLS decrypt length guard to prevent padding-strip underflow/overread in DES3 TLS decrypt. |
| src/wp_dec_pem2der.c | Adds passphrase length caps and rejects short/truncated PEM inputs before fixed-length header checks. |
| src/wp_dec_epki2pki.c | Guards sz * 2 allocations/encodes against overflow and caps password length from callbacks. |
| src/wp_aes_wrap.c | Guards AES key-wrap lengths against word32 truncation. |
| src/wp_aes_block.c | Fixes TLS CBC decrypt record-base pointer usage and clears output length on bad padding. |
| src/wp_aes_aead.c | Resets CCM TLS AAD length on init and per record to prevent stale-AAD reuse. |
Comments suppressed due to low confidence (1)
src/wp_params.c:468
- wp_params_get_utf8_string still risks an out-of-bounds read when the incoming OSSL_PARAM UTF8 string is not NUL-terminated (as allowed by OSSL_PARAM via data_size). XSTRNCPY/strncpy-style copies may read past p->data_size while searching for a terminator. Copy only p->data_size bytes (minus an optional trailing NUL), then add your own terminator in the destination buffer.
if ((p != NULL) && ok && (p->data_size >= len)) {
ok = 0;
}
if ((p != NULL) && ok) {
XSTRNCPY(str, p->data, len);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Uh oh!
There was an error while loading. Please reload this page.