Skip to content

Commit 1d0ae32

Browse files
committed
Coverity: Argument cannot be negative
1. Due to not checking the result of fseek(), it is possible to try to malloc() -1 bytes of storage. Checking the return from fseek() and erring if negative. 2. Changing the check between the result of fseek() and fread() to match signedness. Adding some casting, as at that point the fseek() result is always positive. Fixes CIDs: 573009 572928 572868
1 parent c641294 commit 1d0ae32

File tree

4 files changed

+38
-23
lines changed

4 files changed

+38
-23
lines changed

apps/wolfssh/common.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ static int load_der_file(const char* filename, byte** out, word32* outSz)
7676
{
7777
WFILE* file;
7878
byte* in;
79-
word32 inSz;
79+
long inSz;
8080
int ret;
8181

8282
if (filename == NULL || out == NULL || outSz == NULL)
@@ -90,10 +90,10 @@ static int load_der_file(const char* filename, byte** out, word32* outSz)
9090
WFCLOSE(NULL, file);
9191
return -1;
9292
}
93-
inSz = (word32)WFTELL(NULL, file);
93+
inSz = WFTELL(NULL, file);
9494
WREWIND(NULL, file);
9595

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

107107
ret = (int)WFREAD(NULL, in, 1, inSz, file);
108-
if (ret <= 0 || (word32)ret != inSz) {
108+
if (ret <= 0 || (word32)ret != (word32)inSz) {
109109
ret = -1;
110110
WFREE(in, NULL, 0);
111111
in = 0;

apps/wolfsshd/wolfsshd.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@ static byte* getBufferFromFile(const char* fileName, word32* bufSz, void* heap)
242242
{
243243
FILE* file;
244244
byte* buf = NULL;
245-
word32 fileSz;
245+
long fileSz;
246246
word32 readSz;
247247

248248
WOLFSSH_UNUSED(heap);
@@ -252,13 +252,17 @@ static byte* getBufferFromFile(const char* fileName, word32* bufSz, void* heap)
252252
if (WFOPEN(NULL, &file, fileName, "rb") != 0)
253253
return NULL;
254254
WFSEEK(NULL, file, 0, WSEEK_END);
255-
fileSz = (word32)WFTELL(NULL, file);
255+
fileSz = WFTELL(NULL, file);
256+
if (fileSz < 0) {
257+
WFCLOSE(NULL, file);
258+
return NULL;
259+
}
256260
WREWIND(NULL, file);
257261

258262
buf = (byte*)WMALLOC(fileSz + 1, heap, DYNTYPE_SSHD);
259263
if (buf != NULL) {
260264
readSz = (word32)WFREAD(NULL, buf, 1, fileSz, file);
261-
if (readSz < fileSz) {
265+
if (readSz < (size_t)fileSz) {
262266
WFCLOSE(NULL, file);
263267
WFREE(buf, heap, DYNTYPE_SSHD);
264268
return NULL;

examples/client/common.c

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ static int load_der_file(const char* filename, byte** out, word32* outSz,
262262
{
263263
WFILE* file;
264264
byte* in;
265-
word32 inSz;
265+
long inSz;
266266
int ret;
267267

268268
if (filename == NULL || out == NULL || outSz == NULL)
@@ -276,13 +276,12 @@ static int load_der_file(const char* filename, byte** out, word32* outSz,
276276
WFCLOSE(NULL, file);
277277
return -1;
278278
}
279-
inSz = (word32)WFTELL(NULL, file);
280-
WREWIND(NULL, file);
281-
282-
if (inSz == 0) {
279+
inSz = WFTELL(NULL, file);
280+
if (inSz <= 0) {
283281
WFCLOSE(NULL, file);
284282
return -1;
285283
}
284+
WREWIND(NULL, file);
286285

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

293292
ret = (int)WFREAD(NULL, in, 1, inSz, file);
294-
if (ret <= 0 || (word32)ret != inSz) {
293+
if (ret <= 0 || (long)ret != inSz) {
295294
ret = -1;
296295
WFREE(in, heap, 0);
297296
in = 0;

tests/api.c

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -504,26 +504,38 @@ static int load_file(const char* filename, byte** buf, word32* bufSz)
504504
}
505505

506506
if (ret == 0) {
507-
fseek(f, 0, XSEEK_END);
508-
*bufSz = (word32)ftell(f);
509-
rewind(f);
507+
ret = fseek(f, 0, XSEEK_END);
508+
if (ret < 0)
509+
ret = -3;
510+
}
511+
512+
if (ret == 0) {
513+
long sz = ftell(f);
514+
if (sz < 0)
515+
ret = -4;
516+
else
517+
*bufSz = (word32)sz;
510518
}
511519

512520
if (ret == 0) {
521+
rewind(f);
513522
*buf = (byte*)malloc(*bufSz);
514523
if (*buf == NULL)
515-
ret = -3;
524+
ret = -5;
516525
}
517526

518527
if (ret == 0) {
519-
int readSz;
520-
readSz = (int)fread(*buf, 1, *bufSz, f);
521-
if (readSz < (int)*bufSz)
522-
ret = -4;
528+
size_t readSz;
529+
readSz = fread(*buf, 1, *bufSz, f);
530+
if (readSz < *bufSz)
531+
ret = -6;
523532
}
524533

525-
if (f != NULL)
526-
fclose(f);
534+
if (f != NULL) {
535+
ret = fclose(f);
536+
if (ret < 0)
537+
ret = -7;
538+
}
527539

528540
return ret;
529541
}

0 commit comments

Comments
 (0)