From 2b0f5ede6ad5e9cedb8580fa8bc4559bc097cc6c Mon Sep 17 00:00:00 2001 From: Mark Atwood Date: Thu, 9 Jul 2026 17:30:55 -0700 Subject: [PATCH] fix: force-zero secret buffers before free 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. --- apps/wolfsshd/wolfsshd.c | 2 ++ src/agent.c | 4 ++-- src/internal.c | 38 +++++++++++++++++++++++++++++++++++++- src/wolfsftp.c | 4 ++++ 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/apps/wolfsshd/wolfsshd.c b/apps/wolfsshd/wolfsshd.c index 0776839d9..b6950037e 100644 --- a/apps/wolfsshd/wolfsshd.c +++ b/apps/wolfsshd/wolfsshd.c @@ -1946,6 +1946,8 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh, } } + ForceZero(channelBuffer, sizeof channelBuffer); + ForceZero(shellBuffer, sizeof shellBuffer); (void)conn; return WS_SUCCESS; } diff --git a/src/agent.c b/src/agent.c index 7b7ac5c5d..bbff623af 100644 --- a/src/agent.c +++ b/src/agent.c @@ -1582,10 +1582,10 @@ void wolfSSH_AGENT_ID_free(WOLFSSH_AGENT_ID* id, void* heap) if (id != NULL) { if (id->keyBuffer != NULL) { - WMEMSET(id->keyBuffer, 0, id->keyBufferSz); + ForceZero(id->keyBuffer, id->keyBufferSz); WFREE(id->keyBuffer, heap, DYNTYPE_STRING); } - WMEMSET(id, 0, sizeof(WOLFSSH_AGENT_ID)); + ForceZero(id, sizeof(WOLFSSH_AGENT_ID)); WFREE(id, heap, DYNTYPE_AGENT_ID); } diff --git a/src/internal.c b/src/internal.c index 8b7fd28cc..436216624 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1558,6 +1558,10 @@ void SshResourceFree(WOLFSSH* ssh, void* heap) HandshakeInfoFree(ssh->handshake, heap); ForceZero(&ssh->keys, sizeof(Keys)); ForceZero(&ssh->peerKeys, sizeof(Keys)); + ForceZero(ssh->h, sizeof(ssh->h)); + ssh->hSz = 0; + ForceZero(ssh->sessionId, sizeof(ssh->sessionId)); + ssh->sessionIdSz = 0; if (ssh->rng) { wc_FreeRng(ssh->rng); WFREE(ssh->rng, heap, DYNTYPE_RNG); @@ -2794,6 +2798,11 @@ static int SetHostPrivateKey(WOLFSSH_CTX* ctx, } if (destIdx >= WOLFSSH_MAX_PVT_KEYS) { + /* CTX is full and keyId is new: der cannot be stored, so this + * function takes ownership and zeroizes+frees it to avoid leaking + * the private key material. */ + ForceZero(der, derSz); + WFREE(der, ctx->heap, dynamicType); ret = WS_CTX_KEY_COUNT_E; } else { @@ -3552,6 +3561,8 @@ void ChannelDelete(WOLFSSH_CHANNEL* channel, void* heap) if (channel->origin) WFREE(channel->origin, heap, DYNTYPE_STRING); #endif /* WOLFSSH_FWD */ + if (channel->inputBuffer.buffer) + ForceZero(channel->inputBuffer.buffer, channel->inputBuffer.bufferSz); WFREE(channel->inputBuffer.buffer, channel->inputBuffer.heap, DYNTYPE_BUFFER); if (channel->command) @@ -6664,6 +6675,9 @@ static int KeyAgreeEcdhMlKem_client(WOLFSSH* ssh, byte hashId, ret); } + /* Zero the ML-KEM private key material in handshake->x now that + * decapsulation is done, matching the DH path's post-use ForceZero. */ + ForceZero(ssh->handshake->x, ssh->handshake->xSz); wc_MlKemKey_Free(&kem); /* Replace the concatenated shared secrets with the hash. That @@ -8274,8 +8288,11 @@ static int DoUserAuthRequestRsa(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, WFREE(key, ssh->ctx->heap, DYNTYPE_PUBKEY); } if (encDigest) { + ForceZero(encDigest, MAX_ENCODED_SIG_SZ); WFREE(encDigest, ssh->ctx->heap, DYNTYPE_BUFFER); } +#else + ForceZero(encDigest, sizeof(encDigest)); #endif WLOG(WS_LOG_DEBUG, "Leaving DoUserAuthRequestRsa(), ret = %d", ret); @@ -8435,8 +8452,11 @@ static int DoUserAuthRequestRsaCert(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, WFREE(key, ssh->ctx->heap, DYNTYPE_PUBKEY); } if (encDigest) { + ForceZero(encDigest, MAX_ENCODED_SIG_SZ); WFREE(encDigest, ssh->ctx->heap, DYNTYPE_BUFFER); } +#else + ForceZero(encDigest, sizeof(encDigest)); #endif WLOG(WS_LOG_DEBUG, "Leaving DoUserAuthRequestRsaCert(), ret = %d", ret); @@ -9477,6 +9497,8 @@ static int DoUserAuthRequestPublicKey(WOLFSSH* ssh, WS_UserAuthData* authData, ret = WS_INVALID_ALGO_ID; } } + + ForceZero(digest, sizeof(digest)); } if (ret != WS_SUCCESS) { @@ -14059,9 +14081,14 @@ static int SignHRsa(WOLFSSH* ssh, byte* sig, word32* sigSz, &sigKey->sk.rsa.key, heap, "SignHRsa"); } + ForceZero(digest, sizeof(digest)); #ifdef WOLFSSH_SMALL_STACK - if (encSig != NULL) + if (encSig != NULL) { + ForceZero(encSig, MAX_ENCODED_SIG_SZ); WFREE(encSig, heap, DYNTYPE_TEMP); + } + #else + ForceZero(encSig, MAX_ENCODED_SIG_SZ); #endif WLOG(WS_LOG_DEBUG, "Leaving SignHRsa(), ret = %d", ret); return ret; @@ -14203,6 +14230,7 @@ static int SignHEcdsa(WOLFSSH* ssh, byte* sig, word32* sigSz, WMEMCPY(sig + idx, s, sSz); } + ForceZero(digest, sizeof(digest)); #ifdef WOLFSSH_SMALL_STACK if (r) WFREE(r, heap, DYNTYPE_BUFFER); @@ -16227,6 +16255,8 @@ static int BuildUserAuthRequestRsa(WOLFSSH* ssh, if (ret == WS_SUCCESS) begin += keySig->sigSz; + + ForceZero(encDigest, sizeof(encDigest)); } } @@ -16238,6 +16268,7 @@ static int BuildUserAuthRequestRsa(WOLFSSH* ssh, WFREE(checkData, ssh->ctx->heap, DYNTYPE_TEMP); } + ForceZero(digest, sizeof(digest)); return ret; } /* END BuildUserAuthRequestRsa */ @@ -16407,6 +16438,8 @@ static int BuildUserAuthRequestRsaCert(WOLFSSH* ssh, if (ret == WS_SUCCESS) begin += keySig->sigSz; + + ForceZero(encDigest, sizeof(encDigest)); } } @@ -16418,6 +16451,7 @@ static int BuildUserAuthRequestRsaCert(WOLFSSH* ssh, WFREE(checkData, ssh->ctx->heap, DYNTYPE_TEMP); } + ForceZero(digest, sizeof(digest)); WLOG(WS_LOG_DEBUG, "Leaving BuildUserAuthRequestRsaCert(), ret = %d", ret); return ret; @@ -16685,6 +16719,7 @@ static int BuildUserAuthRequestEcc(WOLFSSH* ssh, WFREE(checkData, ssh->ctx->heap, DYNTYPE_TEMP); } + ForceZero(digest, sizeof(digest)); #ifdef WOLFSSH_SMALL_STACK if (r_ptr) WFREE(r_ptr, ssh->ctx->heap, DYNTYPE_BUFFER); @@ -16945,6 +16980,7 @@ static int BuildUserAuthRequestEccCert(WOLFSSH* ssh, WFREE(checkData, ssh->ctx->heap, DYNTYPE_TEMP); } + ForceZero(digest, sizeof(digest)); return ret; } diff --git a/src/wolfsftp.c b/src/wolfsftp.c index 9d4671834..a31b5b4b0 100644 --- a/src/wolfsftp.c +++ b/src/wolfsftp.c @@ -850,6 +850,7 @@ static void wolfSSH_SFTP_ClearState(WOLFSSH* ssh, enum WS_SFTP_STATE_ID state) if (state & STATE_ID_GET) { if (ssh->getState) { + ForceZero(ssh->getState, sizeof(WS_SFTP_GET_STATE)); WFREE(ssh->getState, ssh->ctx->heap, DYNTYPE_SFTP_STATE); ssh->getState = NULL; } @@ -928,6 +929,7 @@ static void wolfSSH_SFTP_ClearState(WOLFSSH* ssh, enum WS_SFTP_STATE_ID state) if (state & STATE_ID_PUT) { if (ssh->putState) { + ForceZero(ssh->putState, sizeof(WS_SFTP_PUT_STATE)); WFREE(ssh->putState, ssh->ctx->heap, DYNTYPE_SFTP_STATE); ssh->putState = NULL; } @@ -9775,6 +9777,7 @@ int wolfSSH_SFTP_Get(WOLFSSH* ssh, char* from, case STATE_GET_CLEANUP: WLOG(WS_LOG_SFTP, "SFTP GET STATE: CLEANUP"); if (ssh->getState != NULL) { + ForceZero(ssh->getState, sizeof(WS_SFTP_GET_STATE)); WFREE(ssh->getState, ssh->ctx->heap, DYNTYPE_SFTP_STATE); ssh->getState = NULL; } @@ -9998,6 +10001,7 @@ int wolfSSH_SFTP_Put(WOLFSSH* ssh, char* from, char* to, byte resume, case STATE_PUT_CLEANUP: WLOG(WS_LOG_SFTP, "SFTP PUT STATE: CLEANUP"); if (ssh->putState != NULL) { + ForceZero(ssh->putState, sizeof(WS_SFTP_PUT_STATE)); WFREE(ssh->putState, ssh->ctx->heap, DYNTYPE_SFTP_STATE); ssh->putState = NULL; }