ssl: fix miscellaneous issues found in review.#10972
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses several small correctness and hardening issues in the wolfSSL OpenSSL-compat/API surface, including safer handling of sensitive buffers, a string-termination fix, and a few API correctness fixes.
Changes:
- Harden RNG-related code paths by wiping sensitive temporary buffers and tightening PRF hash selection.
- Fix OID numeric string conversion to reliably reserve space for NUL termination.
- Correct API tracing/logging and memory free behavior, and add a missing allocation failure check.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/ssl_crypto.c | Adds entropy/secret buffer zeroization and adjusts PRF hash selection logic for wolfSSL_RAND_pseudo_bytes/wolfSSL_RAND_poll. |
| src/ssl_asn1.c | Fixes an off-by-one risk by reserving space for NUL termination when decoding OIDs into strings. |
| src/ssl_api_rw.c | Fixes an incorrect WOLFSSL_ENTER() trace string in wolfSSL_SendUserCanceled. |
| src/ssl_api_pk.c | Frees ctx->srp_password using the correct heap (ctx->heap). |
| src/ssl_api_cert.c | Adds a NULL check for wolfSSL_sk_X509_new_null() allocation failure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
Retest this please. (passed then aborted, no logs). |
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10972
Scan targets checked: wolfssl-bugs, wolfssl-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
| return NULL; | ||
|
|
||
| sk = wolfSSL_sk_X509_new_null(); | ||
| if (sk == NULL) { |
There was a problem hiding this comment.
🔵 [Low] Dead sk == NULL check after new early-return guard · Dead error handling
The PR adds an early return NULL when wolfSSL_sk_X509_new_null() returns NULL (line 2839), making the pre-existing if (sk == NULL) check at line 2870 unreachable — sk is never reassigned after line 2838.
Fix: Remove the dead if (sk == NULL) block at line 2870, or replace it with a WOLFSSL_ASSERT if the invariant is worth documenting.
There was a problem hiding this comment.
silly oversight on my part, fixed.
Description
Fixes some pre-existing issues found by copilot while reviewing another PR (#10926).
bufLen == MAX_OID_STRING_SZ).#else #errorclause if no hash function available.secretandentropy.Testing
multi-test: