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
+58 −79
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
801ac07
Non-constant-time password hash comparison
ejohnstown Mar 9, 2026
a8ad9f0
DoIgnore Missing Payload Bounds Validation
ejohnstown Mar 9, 2026
9cb60cd
DoUserAuthRequestPassword Missing Bounds Check
ejohnstown Mar 9, 2026
6638c01
DoServiceRequest Missing Bounds Check
ejohnstown Mar 9, 2026
6a3e97d
PrepareUserAuthRequestEcc Missing Bounds Checks
ejohnstown Mar 9, 2026
331048d
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((const byte*)hashedInput,
(const 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
117 changes: 47 additions & 70 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3575,12 +3575,12 @@ int GetSkip(const byte* buf, word32 len, word32* idx)
int result;
word32 sz;

result = GetUint32(&sz, buf, len, idx);
result = GetSize(&sz, buf, len, idx);

if (result == WS_SUCCESS) {
result = WS_BUFFER_E;

if (*idx < len && sz <= len - *idx) {
if (*idx <= len && sz <= len - *idx) {
*idx += sz;
result = WS_SUCCESS;
}
Expand Down 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+1];
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+1];
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,32 @@ 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) {
WLOG(WS_LOG_ERROR,
"wc_ecc_import_x963 failed, ret = %d", ret);
ret = WS_ECC_E;
}
else {
ret = WS_SUCCESS;
}
}
else
#endif
Expand Down
15 changes: 8 additions & 7 deletions src/wolfterm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,15 @@ static void wolfSSH_ClearScreen(WOLFSSH_HANDLE handle, word32 x1, word32 y1, wor
start.Y = y1;

/* get number of cells */
if (y1 == y2) { /* on same line so is x2 - x1 */
fill = x2 - x1;
if (y2 == y1) { /* on same line so is x2 - x1 */
fill = (x2 >= x1) ? (x2 - x1) : 0;
}
else { /* | y1 - y2 | * maxX - x1 + x2 */
fill = y1 - y2;
if (fill < 0)
fill += fill * 2;
fill = fill * maxX - x1 + x2;
/* (y2 - y1) * maxX - x1 + x2 */
else if (y2 > y1) {
fill = (y2 - y1) * maxX - x1 + x2;
}
else {
fill = 0;
}

FillConsoleOutputCharacterA(handle, ' ', fill, start, &w);
Expand Down
Loading