F 569 : Fix stack buffer overflow in encryption setup#212
F 569 : Fix stack buffer overflow in encryption setup#212miyazakh wants to merge 4 commits intowolfSSL:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes a stack buffer overflow in the encryption CLI setup by replacing unbounded scanf("%s", ...) reads with bounded fgets() reads, and adds regression tests to validate stdin-driven input/output filename paths.
Changes:
- Replace unbounded
scanfwith boundedfgetsfor-in/-outprompts in encryption setup. - Add regression tests that supply input/output filenames via stdin (including a non-EVP path probe).
- Expand an OCSP interop test’s expected error-message matching.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tests/ocsp/ocsp-interop-test.sh | Broadens grep pattern to recognize more “missing file” error variants. |
| tests/encrypt/enc-test.sh | Adds stdin-based regression tests covering the new fgets() input handling. |
| src/x509/clu_x509_sign.c | Minor formatting cleanup + extends hash-type switch cases under a version gate. |
| src/crypto/clu_crypto_setup.c | Replaces unsafe scanf("%s") reads with fgets() and newline trimming. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9b1c8c5 to
2a009f1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| wolfCLU_LogError("input too long, please try again"); | ||
| continue; | ||
| } | ||
| inName[strcspn(inName, "\n")] = '\0'; |
There was a problem hiding this comment.
Stripping only \"\\n\" leaves a trailing \"\\r\" on CRLF inputs (common on Windows), which can make filenames fail to open. Use a delimiter set like \"\\r\\n\" here (and similarly for outNameEnc/outNameDec).
| inName[strcspn(inName, "\n")] = '\0'; | |
| inName[strcspn(inName, "\r\n")] = '\0'; |
| } | ||
| inName[strcspn(inName, "\n")] = '\0'; | ||
| /* Do not accept an empty string as valid input */ | ||
| if (inName[0] == '\0') { |
There was a problem hiding this comment.
Empty input is silently ignored and the loop re-prompts without any feedback. Consider logging a short message (or re-printing the prompt with a clear hint) so interactive users understand why it’s asking again. Same applies to the empty-string checks for outNameEnc/outNameDec.
| if (inName[0] == '\0') { | |
| if (inName[0] == '\0') { | |
| wolfCLU_LogError("empty input is not allowed, please try again"); |
|
|
||
| # Regression tests for stack buffer overflow fix (scanf -> fgets) | ||
|
|
||
| # Test: -in not provided, filename supplied via stdin to exercise the inName Path |
There was a problem hiding this comment.
These tests assume the output files don’t already exist. If ./wolfssl enc prompts/refuses on overwrite (or if a previous run left artifacts), the test can hang or behave inconsistently. Consider rm -f of the target outputs before invoking ./wolfssl enc for each case to make runs idempotent.
| # Test: -in not provided, filename supplied via stdin to exercise the inName Path | |
| # Test: -in not provided, filename supplied via stdin to exercise the inName Path | |
| rm -f test-stdin-in.enc test-stdin-in.dec |
| echo "Failed: enc with stdin input (no -in flag)" | ||
| exit 99 | ||
| fi | ||
| ./wolfssl enc -d -aes-128-cbc -in test-stdin-in.enc -out test-stdin-in.dec -k "testpass" > /dev/null 2>&1 |
There was a problem hiding this comment.
These tests assume the output files don’t already exist. If ./wolfssl enc prompts/refuses on overwrite (or if a previous run left artifacts), the test can hang or behave inconsistently. Consider rm -f of the target outputs before invoking ./wolfssl enc for each case to make runs idempotent.
| echo "Failed: stdin enc/dec roundtrip mismatch" | ||
| exit 99 | ||
| fi | ||
| rm -f test-stdin-in.enc test-stdin-in.dec |
There was a problem hiding this comment.
These tests assume the output files don’t already exist. If ./wolfssl enc prompts/refuses on overwrite (or if a previous run left artifacts), the test can hang or behave inconsistently. Consider rm -f of the target outputs before invoking ./wolfssl enc for each case to make runs idempotent.
Fix stack buffer overflow via unbounded scanf in encryption setup
Add test coverage
Depends on: #211 (Merged)
Depends on: #219