From 06c13cdeecd371080bcaed0109caaf16071d01ed Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 9 Mar 2026 10:13:29 -0700 Subject: [PATCH 1/6] Non-constant-time password hash comparison In wolfSSHd, the comparisons of the password hash and public keys were using memcmp(). Changed to use ConstantCompare(). Affected functions: CheckPasswordHashUnix, CheckPublicKeyUnix. Issue: F-53 --- apps/wolfsshd/auth.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/wolfsshd/auth.c b/apps/wolfsshd/auth.c index 1b13d6af1..af3a70392 100644 --- a/apps/wolfsshd/auth.c +++ b/apps/wolfsshd/auth.c @@ -338,7 +338,8 @@ static int CheckPasswordHashUnix(const char* input, char* stored) if (storedSz == 0 || stored[0] == '*' || hashedInputSz == 0 || hashedInput[0] == '*' || hashedInputSz != storedSz || - WMEMCMP(hashedInput, stored, storedSz) != 0) { + ConstantCompare((byte*)hashedInput, + (byte*)stored, storedSz) != 0) { ret = WSSHD_AUTH_FAILURE; } } @@ -656,7 +657,7 @@ static int CheckPublicKeyUnix(const char* name, if (rc == WS_SUCCESS) { rc = wc_Hash(WC_HASH_TYPE_SHA256, caKey, caKeySz, fingerprint, WC_SHA256_DIGEST_SIZE); - if (rc == 0 && WMEMCMP(fingerprint, pubKeyCtx->caKey, + if (rc == 0 && ConstantCompare(fingerprint, pubKeyCtx->caKey, WC_SHA256_DIGEST_SIZE) == 0) { foundKey = 1; break; From 700132935f87dba3eafb3cb4a1de835677aa7e89 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 9 Mar 2026 10:19:59 -0700 Subject: [PATCH 2/6] DoIgnore Missing Payload Bounds Validation The DoIgnore() function was not bounds checking the ignore message. Changed it to use the GetSkip() function which does bounds checking and skips the current blob. Affected function: DoIgnore. Issue: F-410 --- src/internal.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/internal.c b/src/internal.c index e40878584..fb3b99012 100644 --- a/src/internal.c +++ b/src/internal.c @@ -6320,18 +6320,8 @@ static int DoKexDhGexGroup(WOLFSSH* ssh, static int DoIgnore(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) { - word32 dataSz; - word32 begin = *idx; - WOLFSSH_UNUSED(ssh); - WOLFSSH_UNUSED(len); - - ato32(buf + begin, &dataSz); - begin += LENGTH_SZ + dataSz; - - *idx = begin; - - return WS_SUCCESS; + return GetSkip(buf, len, idx); } static int DoRequestSuccess(WOLFSSH *ssh, byte *buf, word32 len, word32 *idx) From cc4db2d6858da00b3f84e3f817eb9cdd9e7f92f8 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 9 Mar 2026 11:10:29 -0700 Subject: [PATCH 3/6] DoUserAuthRequestPassword Missing Bounds Check Replace the original message parsing functions with the GetStringRef() function, which does better bounds checking. Affected function: DoUserAuthRequestPassword. Issue: F-411 --- src/internal.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/internal.c b/src/internal.c index fb3b99012..091f5c282 100644 --- a/src/internal.c +++ b/src/internal.c @@ -6890,20 +6890,14 @@ static int DoUserAuthRequestPassword(WOLFSSH* ssh, WS_UserAuthData* authData, } if (ret == WS_SUCCESS) - ret = GetUint32(&pw->passwordSz, buf, len, &begin); + ret = GetStringRef(&pw->passwordSz, &pw->password, buf, len, &begin); if (ret == WS_SUCCESS) { - pw->password = buf + begin; - begin += pw->passwordSz; - if (pw->hasNewPassword) { /* Skip the password change. Maybe error out since we aren't * supporting password changes at this time. */ - ret = GetUint32(&pw->newPasswordSz, buf, len, &begin); - if (ret == WS_SUCCESS) { - pw->newPassword = buf + begin; - begin += pw->newPasswordSz; - } + ret = GetStringRef(&pw->newPasswordSz, &pw->newPassword, + buf, len, &begin); } else { pw->newPassword = NULL; From 6a89725ef74124c3efad3cc617fa8ee4f43f15f6 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 9 Mar 2026 13:46:54 -0700 Subject: [PATCH 4/6] DoServiceRequest Missing Bounds Check Replace the original message parsing functions with the GetString() function, which does better bounds checking. Affected functions: DoServiceRequest, DoServiceAccept. Issue: F-524, F-525 --- src/internal.c | 52 ++++++++++++++++---------------------------------- 1 file changed, 16 insertions(+), 36 deletions(-) diff --git a/src/internal.c b/src/internal.c index 091f5c282..89aa5d5c2 100644 --- a/src/internal.c +++ b/src/internal.c @@ -6523,56 +6523,36 @@ static int DoDisconnect(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) static int DoServiceRequest(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) { - word32 begin = *idx; - word32 nameSz; - char serviceName[WOLFSSH_MAX_NAMESZ]; - - WOLFSSH_UNUSED(len); + char name[WOLFSSH_MAX_NAMESZ]; + word32 nameSz = sizeof(name); + int ret; - ato32(buf + begin, &nameSz); - begin += LENGTH_SZ; + ret = GetString(name, &nameSz, buf, len, idx); - if (begin + nameSz > len || nameSz >= WOLFSSH_MAX_NAMESZ) { - return WS_BUFFER_E; + if (ret == WS_SUCCESS) { + WLOG(WS_LOG_DEBUG, "Requesting service: %s", name); + ssh->clientState = CLIENT_USERAUTH_REQUEST_DONE; } - WMEMCPY(serviceName, buf + begin, nameSz); - begin += nameSz; - serviceName[nameSz] = 0; - - *idx = begin; - - WLOG(WS_LOG_DEBUG, "Requesting service: %s", serviceName); - ssh->clientState = CLIENT_USERAUTH_REQUEST_DONE; - - return WS_SUCCESS; + return ret; } static int DoServiceAccept(WOLFSSH* ssh, byte* buf, word32 len, word32* idx) { - word32 begin = *idx; - word32 nameSz; - char serviceName[WOLFSSH_MAX_NAMESZ]; + char name[WOLFSSH_MAX_NAMESZ]; + word32 nameSz = sizeof(name); + int ret; - ato32(buf + begin, &nameSz); - begin += LENGTH_SZ; + ret = GetString(name, &nameSz, buf, len, idx); - if (begin + nameSz > len || nameSz >= WOLFSSH_MAX_NAMESZ) { - return WS_BUFFER_E; + if (ret == WS_SUCCESS) { + WLOG(WS_LOG_DEBUG, "Accepted service: %s", name); + ssh->serverState = SERVER_USERAUTH_REQUEST_DONE; } - WMEMCPY(serviceName, buf + begin, nameSz); - begin += nameSz; - serviceName[nameSz] = 0; - - *idx = begin; - - WLOG(WS_LOG_DEBUG, "Accepted service: %s", serviceName); - ssh->serverState = SERVER_USERAUTH_REQUEST_DONE; - - return WS_SUCCESS; + return ret; } From d8ff2a52d7f6aae600438c455f74d1e2b5622603 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 9 Mar 2026 14:06:22 -0700 Subject: [PATCH 5/6] PrepareUserAuthRequestEcc Missing Bounds Checks For agent ECC public key parsing, replaced parsing the data by hand with the GetSkip() and GetStringRef() functions which do bounds checking. Affected function: PrepareUserAuthRequestEcc. Issue: F-526 --- src/internal.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/internal.c b/src/internal.c index 89aa5d5c2..070226e25 100644 --- a/src/internal.c +++ b/src/internal.c @@ -14400,19 +14400,27 @@ static int PrepareUserAuthRequestEcc(WOLFSSH* ssh, word32* payloadSz, word32 idx = 0; #ifdef WOLFSSH_AGENT if (ssh->agentEnabled) { - word32 sz; - const byte* c = (const byte*)authData->sf.publicKey.publicKey; - - ato32(c + idx, &sz); - idx += LENGTH_SZ + sz; - ato32(c + idx, &sz); - idx += LENGTH_SZ + sz; - ato32(c + idx, &sz); - idx += LENGTH_SZ; - c += idx; - idx = 0; + const byte* publicKey = NULL; + word32 publicKeySz; - ret = wc_ecc_import_x963(c, sz, &keySig->ks.ecc.key); + ret = GetSkip((const byte*)authData->sf.publicKey.publicKey, + authData->sf.publicKey.publicKeySz, &idx); + if (ret == WS_SUCCESS) { + ret = GetSkip((const byte*)authData->sf.publicKey.publicKey, + authData->sf.publicKey.publicKeySz, &idx); + } + if (ret == WS_SUCCESS) { + ret = GetStringRef(&publicKeySz, &publicKey, + (const byte*)authData->sf.publicKey.publicKey, + authData->sf.publicKey.publicKeySz, &idx); + } + if (ret == WS_SUCCESS) { + ret = wc_ecc_import_x963(publicKey, publicKeySz, + &keySig->ks.ecc.key); + } + if (ret == 0) { + ret = WS_SUCCESS; + } } else #endif From e643e69fd92ecf6b81cef2ab691ebe3947d5c149 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 9 Mar 2026 14:16:57 -0700 Subject: [PATCH 6/6] Unsigned variable compared < 0 When filling the screen with spaces, the code was subtracting two unsigned numbers and checking if they were negative. Changed to use a comparison and adjust the subtraction as appropriate, then did the rest of the size expansion. Affected function: wolfSSH_ClearScreen. Issue: F-48 --- src/wolfterm.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/wolfterm.c b/src/wolfterm.c index 5fd23bf8f..65bef63e1 100644 --- a/src/wolfterm.c +++ b/src/wolfterm.c @@ -119,10 +119,7 @@ static void wolfSSH_ClearScreen(WOLFSSH_HANDLE handle, word32 x1, word32 y1, wor fill = x2 - x1; } else { /* | y1 - y2 | * maxX - x1 + x2 */ - fill = y1 - y2; - if (fill < 0) - fill += fill * 2; - fill = fill * maxX - x1 + x2; + fill = ((y1 > y2) ? y1 - y2 : y2 - y1) * maxX - x1 + x2; } FillConsoleOutputCharacterA(handle, ' ', fill, start, &w);