fixes for closing fd, sanity check init calls, buffer scope, sanity check argmunts passed in#223
Open
JacobBarthelmeh wants to merge 2 commits intowolfSSL:mainfrom
Open
fixes for closing fd, sanity check init calls, buffer scope, sanity check argmunts passed in#223JacobBarthelmeh wants to merge 2 commits intowolfSSL:mainfrom
JacobBarthelmeh wants to merge 2 commits intowolfSSL:mainfrom
Conversation
JacobBarthelmeh
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens wolfCLU against a handful of crash/over-read scenarios caused by missing/overlong CLI arguments and lifetime/cleanup issues, and adds regression tests to ensure these cases fail gracefully.
Changes:
- Add Python regression tests for
ecparam -nameoverlong inputs andgenkeyinvoked without a key-type argument. - Fix/mitigate multiple robustness issues in CLI code paths (argv bounds check in
genkey, NUL-termination forecparamcurve names, stdin-to-BIO buffer lifetime forpkcs8, safer string concatenation in CSR extension printing). - Improve cleanup/defensive behavior in keygen callbacks and DH/DSA setup teardown, and avoid writing output when decrypt length validation fails.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/pkey/ecparam-test.py | Adds regression test ensuring overlong curve names don’t crash. |
| tests/genkey_sign_ver/genkey-sign-ver-test.py | Adds regression test ensuring wolfssl genkey without key type doesn’t crash. |
| src/x509/clu_request_setup.c | Fixes XSTRLCAT usage to use destination buffer size consistently. |
| src/pkcs/clu_pkcs8.c | Fixes stdin key buffer lifetime by heap-allocating and wiping on exit. |
| src/genkey/clu_genkey_setup.c | Adds an argc guard to avoid dereferencing argv[2] when key type is missing. |
| src/genkey/clu_genkey.c | Improves der buffer cleanup before realloc; closes files on XMSS error paths. |
| src/ecparam/clu_ecparam.c | Ensures copied curve name is always NUL-terminated to avoid over-reads. |
| src/dsa/clu_dsa.c | Tracks init state so only initialized objects are freed. |
| src/dh/clu_dh.c | Tracks init state so only initialized objects are freed. |
| src/crypto/clu_decrypt.c | Avoids writing output when an invalid decrypted length is detected. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+183
to
+184
| wolfCLU_LogError("Unable to open pkcs8 file %s", | ||
| optarg); |
Comment on lines
+1440
to
+1445
| /* free any prior derBuf (from PRIV path or initial alloc) | ||
| * before reallocating for the public key */ | ||
| if (derBuf != NULL) { | ||
| wolfCLU_ForceZero(derBuf, keySz); | ||
| XFREE(derBuf, HEAP_HINT, DYNAMIC_TYPE_TMP_BUFFER); | ||
| derBuf = NULL; |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #223
Scan targets checked: wolfclu-bugs, wolfclu-src
No new issues found in the changed files. ✅
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.