Skip to content

Fixing a batch of issues reported by Coverity #820

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

Merged
JacobBarthelmeh merged 5 commits into from
Jul 21, 2025
Merged

Fixing a batch of issues reported by Coverity #820

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cc83fc3
Coverity: String not null terminated
ejohnstown Jul 14, 2025
f24c3c2
Coverity: Dereference before null check
ejohnstown Jul 14, 2025
4700799
Coverity: Dereference before null check
ejohnstown Jul 14, 2025
c641294
Coverity: Buffer not null terminated
ejohnstown Jul 14, 2025
903bbc7
Coverity: Argument cannot be negative
ejohnstown Jul 21, 2025
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
10 changes: 5 additions & 5 deletions apps/wolfssh/common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ static int load_der_file(const char* filename, byte** out, word32* outSz)
{
WFILE* file;
byte* in;
word32 inSz;
long inSz;
int ret;

if (filename == NULL || out == NULL || outSz == NULL)
Expand All @@ -90,10 +90,10 @@ static int load_der_file(const char* filename, byte** out, word32* outSz)
WFCLOSE(NULL, file);
return -1;
}
inSz = (word32)WFTELL(NULL, file);
inSz = WFTELL(NULL, file);
WREWIND(NULL, file);

if (inSz == 0) {
if (inSz <= 0) {
WFCLOSE(NULL, file);
return -1;
}
Expand All @@ -105,7 +105,7 @@ static int load_der_file(const char* filename, byte** out, word32* outSz)
}

ret = (int)WFREAD(NULL, in, 1, inSz, file);
if (ret <= 0 || (word32)ret != inSz) {
if (ret <= 0 || ret != inSz) {
ret = -1;
WFREE(in, NULL, 0);
in = 0;
Expand All @@ -115,7 +115,7 @@ static int load_der_file(const char* filename, byte** out, word32* outSz)
ret = 0;

*out = in;
*outSz = inSz;
*outSz = (word32)inSz;

WFCLOSE(NULL, file);

Expand Down
35 changes: 19 additions & 16 deletions apps/wolfsshd/wolfsshd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ static byte* getBufferFromFile(const char* fileName, word32* bufSz, void* heap)
{
FILE* file;
byte* buf = NULL;
word32 fileSz;
long fileSz;
word32 readSz;

WOLFSSH_UNUSED(heap);
Expand All @@ -252,13 +252,17 @@ static byte* getBufferFromFile(const char* fileName, word32* bufSz, void* heap)
if (WFOPEN(NULL, &file, fileName, "rb") != 0)
return NULL;
WFSEEK(NULL, file, 0, WSEEK_END);
fileSz = (word32)WFTELL(NULL, file);
fileSz = WFTELL(NULL, file);
if (fileSz < 0) {
WFCLOSE(NULL, file);
return NULL;
}
WREWIND(NULL, file);

buf = (byte*)WMALLOC(fileSz + 1, heap, DYNTYPE_SSHD);
if (buf != NULL) {
readSz = (word32)WFREAD(NULL, buf, 1, fileSz, file);
if (readSz < fileSz) {
if (readSz < (size_t)fileSz) {
WFCLOSE(NULL, file);
WFREE(buf, heap, DYNTYPE_SSHD);
return NULL;
Expand Down Expand Up @@ -1347,20 +1351,19 @@ static int SHELL_Subsystem(WOLFSSHD_CONNECTION* conn, WOLFSSH* ssh,
setenv("LOGNAME", pPasswd->pw_name, 1);
setenv("SHELL", pPasswd->pw_shell, 1);

if (pPasswd->pw_shell) {
if (WSTRLEN(pPasswd->pw_shell) < sizeof(shell)) {
char* cursor;
char* start;
if (WSTRLEN(pPasswd->pw_shell) < sizeof(shell)) {
char* cursor;
char* start;

WSTRNCPY(shell, pPasswd->pw_shell, sizeof(shell));
cursor = shell;
do {
start = cursor;
*cursor = '-';
cursor = WSTRCHR(start, '/');
} while (cursor && *cursor != '\0');
args[0] = start;
}
WSTRNCPY(shell, pPasswd->pw_shell, sizeof(shell)-1);
shell[sizeof(shell)-1] = 0;
cursor = shell;
do {
start = cursor;
*cursor = '-';
cursor = WSTRCHR(start, '/');
} while (cursor && *cursor != '\0');
args[0] = start;
}

rc = chdir(pPasswd->pw_dir);
Expand Down
13 changes: 6 additions & 7 deletions examples/client/common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ static int load_der_file(const char* filename, byte** out, word32* outSz,
{
WFILE* file;
byte* in;
word32 inSz;
long inSz;
int ret;

if (filename == NULL || out == NULL || outSz == NULL)
Expand All @@ -276,13 +276,12 @@ static int load_der_file(const char* filename, byte** out, word32* outSz,
WFCLOSE(NULL, file);
return -1;
}
inSz = (word32)WFTELL(NULL, file);
WREWIND(NULL, file);

if (inSz == 0) {
inSz = WFTELL(NULL, file);
if (inSz <= 0) {
WFCLOSE(NULL, file);
return -1;
}
WREWIND(NULL, file);

in = (byte*)WMALLOC(inSz, heap, 0);
if (in == NULL) {
Expand All @@ -291,7 +290,7 @@ static int load_der_file(const char* filename, byte** out, word32* outSz,
}

ret = (int)WFREAD(NULL, in, 1, inSz, file);
if (ret <= 0 || (word32)ret != inSz) {
if (ret <= 0 || ret != inSz) {
ret = -1;
WFREE(in, heap, 0);
in = 0;
Expand All @@ -301,7 +300,7 @@ static int load_der_file(const char* filename, byte** out, word32* outSz,
ret = 0;

*out = in;
*outSz = inSz;
*outSz = (word32)inSz;

WFCLOSE(NULL, file);

Expand Down
82 changes: 43 additions & 39 deletions examples/sftpclient/sftpclient.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,15 @@ static void sig_handler(const int sig)
static void clean_path(char* path)
{
int i;
long sz = (long)WSTRLEN(path);
long sz;
byte found;

if (path == NULL) {
return;
}

sz = (long)WSTRLEN(path);

/* remove any double '/' chars */
for (i = 0; i < sz; i++) {
if (path[i] == '/' && path[i+1] == '/') {
Expand All @@ -272,51 +278,49 @@ static void clean_path(char* path)
}
}

if (path != NULL) {
/* go through path until no cases are found */
do {
int prIdx = 0; /* begin of cut */
int enIdx = 0; /* end of cut */
sz = (long)WSTRLEN(path);

found = 0;
for (i = 0; i < sz; i++) {
if (path[i] == '/') {
int z;

/* if next two chars are .. then delete */
if (path[i+1] == '.' && path[i+2] == '.') {
enIdx = i + 3;

/* start at one char before / and retrace path */
for (z = i - 1; z > 0; z--) {
if (path[z] == '/') {
prIdx = z;
break;
}
/* go through path until no cases are found */
do {
int prIdx = 0; /* begin of cut */
int enIdx = 0; /* end of cut */
sz = (long)WSTRLEN(path);

found = 0;
for (i = 0; i < sz; i++) {
if (path[i] == '/') {
int z;

/* if next two chars are .. then delete */
if (path[i+1] == '.' && path[i+2] == '.') {
enIdx = i + 3;

/* start at one char before / and retrace path */
for (z = i - 1; z > 0; z--) {
if (path[z] == '/') {
prIdx = z;
break;
}
}

/* cut out .. and previous */
WMEMMOVE(path + prIdx, path + enIdx, sz - enIdx);
path[sz - (enIdx - prIdx)] = '\0';

if (enIdx == sz) {
path[prIdx] = '\0';
}
/* cut out .. and previous */
WMEMMOVE(path + prIdx, path + enIdx, sz - enIdx);
path[sz - (enIdx - prIdx)] = '\0';

/* case of at / */
if (WSTRLEN(path) == 0) {
path[0] = '/';
path[1] = '\0';
}
if (enIdx == sz) {
path[prIdx] = '\0';
}

found = 1;
break;
/* case of at / */
if (WSTRLEN(path) == 0) {
path[0] = '/';
path[1] = '\0';
}

found = 1;
break;
}
}
} while (found);
}
}
} while (found);
}

#define WS_MAX_EXAMPLE_RW 1024
Expand Down
6 changes: 5 additions & 1 deletion src/ssh.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1658,9 +1658,13 @@ static int DoSshPubKey(const byte* in, word32 inSz, byte** out,
/*
SSH format is:
type AAAABASE64ENCODEDKEYDATA comment

allocate a copy to tokenize, add a null terminator.
*/
c = WSTRDUP((const char*)in, heap, DYNTYPE_STRING);
c = (char*)WMALLOC(inSz + 1, heap, DYNTYPE_STRING);
if (c != NULL) {
WMEMCPY(c, in, inSz);
c[inSz-1] = 0;
type = WSTRTOK(c, " \n", &last);
key = WSTRTOK(NULL, " \n", &last);
}
Expand Down
32 changes: 22 additions & 10 deletions tests/api.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,26 +504,38 @@ static int load_file(const char* filename, byte** buf, word32* bufSz)
}

if (ret == 0) {
fseek(f, 0, XSEEK_END);
*bufSz = (word32)ftell(f);
rewind(f);
ret = fseek(f, 0, XSEEK_END);
if (ret < 0)
ret = -3;
}

if (ret == 0) {
long sz = ftell(f);
if (sz < 0)
ret = -4;
else
*bufSz = (word32)sz;
}

if (ret == 0) {
rewind(f);
*buf = (byte*)malloc(*bufSz);
if (*buf == NULL)
ret = -3;
ret = -5;
}

if (ret == 0) {
int readSz;
readSz = (int)fread(*buf, 1, *bufSz, f);
if (readSz < (int)*bufSz)
ret = -4;
size_t readSz;
readSz = fread(*buf, 1, *bufSz, f);
if (readSz < *bufSz)
ret = -6;
}

if (f != NULL)
fclose(f);
if (f != NULL) {
ret = fclose(f);
if (ret < 0)
ret = -7;
}

return ret;
}
Expand Down
Loading