Skip to content

Static analysis fixes #892

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ejohnstown wants to merge 6 commits into
base: master
Choose a base branch
from
+44 −74
Open

Static analysis fixes #892

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
06c13cd
Non-constant-time password hash comparison
ejohnstown Mar 9, 2026
7001329
DoIgnore Missing Payload Bounds Validation
ejohnstown Mar 9, 2026
cc4db2d
DoUserAuthRequestPassword Missing Bounds Check
ejohnstown Mar 9, 2026
6a89725
DoServiceRequest Missing Bounds Check
ejohnstown Mar 9, 2026
d8ff2a5
PrepareUserAuthRequestEcc Missing Bounds Checks
ejohnstown Mar 9, 2026
e643e69
Unsigned variable compared < 0
ejohnstown Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions apps/wolfsshd/auth.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
Expand Down
108 changes: 40 additions & 68 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -6533,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;
}


Expand Down Expand Up @@ -6900,20 +6870,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;
Expand Down Expand Up @@ -14436,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
Expand Down
5 changes: 1 addition & 4 deletions src/wolfterm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading