From 46cd6a7d6eab81fd6aa068641d3e86ce3b7122ac Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 23 Feb 2026 09:28:08 +0000 Subject: [PATCH 1/7] Fix logical operator in public key type validation checks Change && to || in 5 instances where public key type matching used AND instead of OR, causing WMEMCMP to be skipped when type sizes matched. Two key types with the same size but different content would incorrectly pass validation. Affected functions: DoUserAuthRequestRsaCert, DoUserAuthRequestEcc, and DoUserAuthRequestEccCert. --- src/internal.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/internal.c b/src/internal.c index a9ad38e25..0c8d8fa37 100644 --- a/src/internal.c +++ b/src/internal.c @@ -7210,8 +7210,9 @@ static int DoUserAuthRequestRsaCert(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, } if (ret == WS_SUCCESS) { - if (publicKeyTypeSz != pk->publicKeyTypeSz && - WMEMCMP(publicKeyType, pk->publicKeyType, publicKeyTypeSz) != 0) { + if (publicKeyTypeSz != pk->publicKeyTypeSz + || WMEMCMP(publicKeyType, pk->publicKeyType, + publicKeyTypeSz) != 0) { WLOG(WS_LOG_DEBUG, "Signature's type does not match public key type"); @@ -7309,8 +7310,9 @@ static int DoUserAuthRequestEcc(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, if (ret == WS_SUCCESS) { publicKeyType = pk->publicKey + i; i += publicKeyTypeSz; - if (publicKeyTypeSz != pk->publicKeyTypeSz && - WMEMCMP(publicKeyType, pk->publicKeyType, publicKeyTypeSz) != 0) { + if (publicKeyTypeSz != pk->publicKeyTypeSz + || WMEMCMP(publicKeyType, pk->publicKeyType, + publicKeyTypeSz) != 0) { WLOG(WS_LOG_DEBUG, "Public Key's type does not match public key type"); @@ -7351,8 +7353,9 @@ static int DoUserAuthRequestEcc(WOLFSSH* ssh, WS_UserAuthData_PublicKey* pk, publicKeyType = pk->signature + i; i += publicKeyTypeSz; - if (publicKeyTypeSz != pk->publicKeyTypeSz && - WMEMCMP(publicKeyType, pk->publicKeyType, publicKeyTypeSz) != 0) { + if (publicKeyTypeSz != pk->publicKeyTypeSz + || WMEMCMP(publicKeyType, pk->publicKeyType, + publicKeyTypeSz) != 0) { WLOG(WS_LOG_DEBUG, "Signature's type does not match public key type"); @@ -7620,7 +7623,7 @@ static int DoUserAuthRequestEd25519(WOLFSSH* ssh, publicKeyType = pk->publicKey + i; i += publicKeyTypeSz; if (publicKeyTypeSz != pk->publicKeyTypeSz - && WMEMCMP(publicKeyType, + || WMEMCMP(publicKeyType, pk->publicKeyType, publicKeyTypeSz) != 0) { WLOG(WS_LOG_DEBUG, "Public Key's type does not match public key type"); @@ -7651,8 +7654,9 @@ static int DoUserAuthRequestEd25519(WOLFSSH* ssh, publicKeyType = pk->signature + i; i += publicKeyTypeSz; - if (publicKeyTypeSz != pk->publicKeyTypeSz && - WMEMCMP(publicKeyType, pk->publicKeyType, publicKeyTypeSz) != 0) { + if (publicKeyTypeSz != pk->publicKeyTypeSz + || WMEMCMP(publicKeyType, pk->publicKeyType, + publicKeyTypeSz) != 0) { WLOG(WS_LOG_DEBUG, "Signature's type does not match public key type"); From 4cb8d1ee9943e02c787d11bc7db3ddc67bcc3c63 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 23 Feb 2026 09:56:01 +0000 Subject: [PATCH 2/7] Fix buffer over-read in wolfSSH_DoModes terminal mode parsing The while loop condition only checked that the opcode byte was in bounds (idx < modesSz) but not the 4-byte argument read by ato32(). When modesSz had a remainder of 1 mod 5 and the trailing byte was a valid opcode (1-159) rather than TTY_OP_END, ato32() would read 4 bytes past the buffer. Change the loop guard to require a full TERMINAL_MODE_SZ bytes remaining before entering the loop body. --- src/internal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/internal.c b/src/internal.c index 0c8d8fa37..f04797fd6 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8944,7 +8944,7 @@ int wolfSSH_DoModes(const byte* modes, word32 modesSz, int fd) tcgetattr(fd, &term); - while (idx < modesSz && modes[idx] != WOLFSSH_TTY_OP_END + while (idx + TERMINAL_MODE_SZ <= modesSz && modes[idx] != WOLFSSH_TTY_OP_END && modes[idx] < WOLFSSH_TTY_INVALID) { ato32(modes + idx + 1, &arg); From 0728d9cac0f05a110a4bb823d58ac749c4d9f212 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 23 Feb 2026 10:12:01 +0000 Subject: [PATCH 3/7] Fix two bugs in PostRemoveId agent identity removal Fix inverted WMEMCMP check that removed non-matching entries and kept matching ones. WMEMCMP returns 0 on match, so the condition was backwards. Fix head-of-list removal setting idList to NULL instead of cur->next, which dropped all remaining entries after the removed one --- src/agent.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/agent.c b/src/agent.c index de918769c..7d97ba563 100644 --- a/src/agent.c +++ b/src/agent.c @@ -590,7 +590,7 @@ static int PostRemoveId(WOLFSSH_AGENT_CTX* agent, int match; match = WMEMCMP(id, cur->id, WC_SHA256_DIGEST_SIZE); - if (!match) { + if (match) { prev = cur; cur = cur->next; } @@ -598,7 +598,7 @@ static int PostRemoveId(WOLFSSH_AGENT_CTX* agent, if (prev != NULL) prev->next = cur->next; else - agent->idList = NULL; + agent->idList = cur->next; wolfSSH_AGENT_ID_free(cur, agent->heap); cur = NULL; From 6547f04a7797e67626b94ab1d341091d10173b6d Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 23 Feb 2026 10:14:21 +0000 Subject: [PATCH 4/7] Fix digest comparison in FindKeyId to use id->id field WMEMCMP was comparing the computed SHA-256 digest against the WOLFSSH_AGENT_ID struct pointer instead of the id field within the struct, causing key lookups by digest to never match. --- src/agent.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/agent.c b/src/agent.c index 7d97ba563..5cc160d64 100644 --- a/src/agent.c +++ b/src/agent.c @@ -660,7 +660,7 @@ static WOLFSSH_AGENT_ID* FindKeyId(WOLFSSH_AGENT_ID* id, if (ret == WS_SUCCESS) { while (id != NULL && - WMEMCMP(digest, id, WC_SHA256_DIGEST_SIZE) != 0 && + WMEMCMP(digest, id->id, WC_SHA256_DIGEST_SIZE) != 0 && WMEMCMP(keyBlob, id->keyBlob, keyBlobSz)) { id = id->next; } From eab0488966f36c242229a9423359e40df222e5d1 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 23 Feb 2026 10:19:25 +0000 Subject: [PATCH 5/7] Fix octal validation loop index in GetScpFileMode The upper-bound check in the octal-to-integer conversion loop used modeOctet[0] instead of modeOctet[i], so only the first character was validated against '7'. Characters at positions 1-3 could have values above '7' without triggering an error. --- src/wolfscp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wolfscp.c b/src/wolfscp.c index 16b8599b0..f9774004a 100644 --- a/src/wolfscp.c +++ b/src/wolfscp.c @@ -854,7 +854,7 @@ static int GetScpFileMode(WOLFSSH* ssh, byte* buf, word32 bufSz, for (i = 0; i < SCP_MODE_OCTET_LEN; i++) { - if (modeOctet[i] < '0' || modeOctet[0] > '7') { + if (modeOctet[i] < '0' || modeOctet[i] > '7') { ret = WS_BAD_ARGUMENT; break; } From 99319bf7733063293ece4bc2c2babc0685fb6e83 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 23 Feb 2026 10:21:35 +0000 Subject: [PATCH 6/7] Fix wrong variable checked in DoCheckUser auth callback In DoCheckUser, after calling auth->checkUserCb(usr) into rc, the failure check on line 1063 compared ret instead of rc against WSSHD_AUTH_FAILURE. Since ret is WOLFSSH_USERAUTH_SUCCESS at that point, the condition was always false, causing callback failures to fall through to the generic error branch with WOLFSSH_USERAUTH_FAILURE instead of returning WOLFSSH_USERAUTH_INVALID_USER. --- apps/wolfsshd/auth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index 0e6a92fda..4a25b480c 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -1060,7 +1060,7 @@ static int DoCheckUser(const char* usr, WOLFSSHD_AUTH* auth) wolfSSH_Log(WS_LOG_INFO, "[SSHD] User ok."); ret = WOLFSSH_USERAUTH_SUCCESS; } - else if (ret == WSSHD_AUTH_FAILURE) { + else if (rc == WSSHD_AUTH_FAILURE) { wolfSSH_Log(WS_LOG_INFO, "[SSHD] User %s doesn't exist.", usr); ret = WOLFSSH_USERAUTH_INVALID_USER; } From 82b9f1138dfa925fdae361eea5fbe67db16b2273 Mon Sep 17 00:00:00 2001 From: Andrew Hutchings Date: Mon, 23 Feb 2026 10:23:26 +0000 Subject: [PATCH 7/7] Fix NULL pointer dereference in wolfSSH_SetTpmDev/SetTpmKey Move the NULL validation checks inside the existing NULL guards for ssh and ssh->ctx. Previously, the check accessed ssh->ctx outside the guard, causing a NULL dereference when ssh or ssh->ctx was NULL. Also fix wolfSSH_SetTpmKey to check tpmKey instead of tpmDev. --- src/ssh.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/ssh.c b/src/ssh.c index c30d5fd4a..1506987e0 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -405,11 +405,12 @@ void wolfSSH_SetTpmDev(WOLFSSH* ssh, WOLFTPM2_DEV* dev) { WLOG(WS_LOG_DEBUG, "Entering wolfSSH_SetTpmDev()"); - if (ssh && ssh->ctx) + if (ssh && ssh->ctx) { ssh->ctx->tpmDev = dev; - if (ssh->ctx->tpmDev == NULL) { - WLOG(WS_LOG_DEBUG, "wolfSSH_SetTpmDev: Set tpm dev failed"); + if (ssh->ctx->tpmDev == NULL) { + WLOG(WS_LOG_DEBUG, "wolfSSH_SetTpmDev: Set tpm dev failed"); + } } } @@ -418,11 +419,12 @@ void wolfSSH_SetTpmKey(WOLFSSH* ssh, WOLFTPM2_KEY* key) { WLOG(WS_LOG_DEBUG, "Entering wolfSSH_SetTpmKey()"); - if (ssh && ssh->ctx) + if (ssh && ssh->ctx) { ssh->ctx->tpmKey = key; - if (ssh->ctx->tpmDev == NULL) { - WLOG(WS_LOG_DEBUG, "wolfSSH_SetTpmKey: Set tpm key failed"); + if (ssh->ctx->tpmKey == NULL) { + WLOG(WS_LOG_DEBUG, "wolfSSH_SetTpmKey: Set tpm key failed"); + } } }