fix: force-zero secret buffers before free#1099
Open
MarkAtwood wants to merge 1 commit into
Open
Conversation
Zeroize key/digest/handshake buffers before they are freed or go out of scope, and fix a private-key leak on the CTX-full reject path. Build-verified with ./configure --enable-all (make exit 0). - #1278 internal.c KeyAgreeEcdhMlKem_client: ForceZero handshake->x ML-KEM private key before wc_MlKemKey_Free, matching the DH path. - #2082 agent.c wolfSSH_AGENT_ID_free: WMEMSET -> ForceZero for the key buffer and the id struct so scrubs are not optimized away. - #2496 internal.c SignHRsa/SignHEcdsa: ForceZero digest (and encSig in SignHRsa) before return. - #2497 internal.c BuildUserAuthRequest Rsa/RsaCert/Ecc/EccCert: ForceZero digest (and encDigest for the RSA paths). - #2498 internal.c DoUserAuthRequestPublicKey: ForceZero digest before it leaves scope on both success and failure exits. - #2500 internal.c DoUserAuthRequestRsa: ForceZero encDigest before free (both SMALL_STACK and stack-array paths). - #2501 internal.c DoUserAuthRequestRsaCert: same encDigest scrub. - #2886 internal.c SshResourceFree: ForceZero ssh->h and ssh->sessionId (and reset sizes) alongside the already-zeroed KDF inputs. - #3453 internal.c SetHostPrivateKey: on CTX-full reject, take ownership and ForceZero+WFREE der instead of leaking the private key. - #3680 internal.c ChannelDelete: ForceZero inputBuffer before free. - #6277 wolfsftp.c ClearState/GET/PUT cleanup: ForceZero SFTP get/put state structs before free (all four sites). - #6278 wolfsshd.c SHELL_Subsystem: ForceZero channelBuffer/shellBuffer on the data-carrying exit.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens wolfSSH’s secret-handling hygiene by ensuring secret-bearing buffers are reliably scrubbed with ForceZero() before being freed or leaving scope, and by fixing a private-key leak on a context-full error path.
Changes:
- Added
ForceZero()scrubbing for authentication digests/signature buffers and key-exchange private-key material insrc/internal.c. - Replaced non-forced zeroing (
WMEMSET) withForceZero()for agent identity buffers insrc/agent.c, and scrubbed plaintext-carrying channel/SFTP state buffers before free. - Ensured wolfsshd subsystem stack buffers are scrubbed on exit.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/internal.c | Scrubs KEX/auth/signature/session materials with ForceZero() and fixes a private-key leak when host-key slots are exhausted. |
| src/agent.c | Uses ForceZero() to scrub agent identity key material and the ID struct before free. |
| src/wolfsftp.c | Scrubs SFTP GET/PUT state structs before freeing them in multiple cleanup paths. |
| apps/wolfsshd/wolfsshd.c | Scrubs subsystem I/O buffers (channelBuffer/shellBuffer) before returning. |
💡 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.
Force-zero hardening across wolfSSH: scrub secret-bearing buffers before they are freed or leave scope, and fix one private-key leak on an error path. All changes build-verified together on Ubuntu 24.04 with
./configure --with-wolfssl=<all> --enable-all && make(exit 0). Reported by Fenrir static analysis.src/internal.cKeyAgreeEcdhMlKem_client— ForceZero the ML-KEM private key inhandshake->xbeforewc_MlKemKey_Free, on both success and error, mirroring the DH path.src/agent.cwolfSSH_AGENT_ID_free— replaceWMEMSETwithForceZerofor the key buffer and the id struct so the compiler cannot elide the scrub.src/internal.cSignHRsa/SignHEcdsa— ForceZerodigest(andencSigin SignHRsa) before return.src/internal.cBuildUserAuthRequest{Rsa,RsaCert,Ecc,EccCert}— ForceZerodigest; alsoencDigest(in-scope) for the two RSA paths.src/internal.cDoUserAuthRequestPublicKey— ForceZerodigestbefore it leaves scope on all exits of the else block.src/internal.cDoUserAuthRequestRsa— ForceZeroencDigestbefore free (SMALL_STACK) / before scope exit (stack array).src/internal.cDoUserAuthRequestRsaCert— sameencDigestscrub.src/internal.cSshResourceFree— ForceZerossh->handssh->sessionId(and reset their sizes) alongside the already-zeroed KDF inputs before free.src/internal.cSetHostPrivateKey— on the CTX-full reject branch, take ownership andForceZero+WFREEderinstead of leaking the private key (no double-free: the store path is unaffected).src/internal.cChannelDelete— ForceZeroinputBuffer.buffer(NULL-guarded) before free to scrub residual plaintext.src/wolfsftp.cwolfSSH_SFTP_ClearStateGET/PUT and the GET/PUT_CLEANUPcases — ForceZero the SFTP get/put state structs before free (all four sites).apps/wolfsshd/wolfsshd.cPOSIXSHELL_Subsystem— ForceZerochannelBuffer/shellBufferon the data-carrying exit.ForceZerois already used throughout these translation units; no new includes.