Skip to content

Commit 0b451e5

Browse files
douzzerdouzzer
authored
Merge pull request wolfSSL#817 from ejohnstown/cov
Fixing a batch of issues reported by Coverity
2 parents dd377d6 + 441f975 commit 0b451e5

File tree

6 files changed

+90
-46
lines changed

6 files changed

+90
-46
lines changed

‎apps/wolfssh/common.c‎

Lines changed: 19 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,8 @@ int ClientUserAuth(byte authType,
499499
{
500500
int ret = WOLFSSH_USERAUTH_SUCCESS;
501501

502+
WOLFSSH_UNUSED(ctx);
503+
502504
#ifdef DEBUG_WOLFSSH
503505
/* inspect supported types from server */
504506
printf("Server supports:\n");
@@ -538,37 +540,28 @@ int ClientUserAuth(byte authType,
538540
ret = WOLFSSH_USERAUTH_SUCCESS;
539541
}
540542
else if (authType == WOLFSSH_USERAUTH_PASSWORD) {
541-
const char* defaultPassword = (const char*)ctx;
542-
word32 passwordSz = 0;
543-
544-
if (defaultPassword != NULL) {
545-
passwordSz = (word32)strlen(defaultPassword);
546-
WMEMCPY(userPassword, defaultPassword, passwordSz);
543+
printf("Password: ");
544+
fflush(stdout);
545+
ClientSetEcho(0);
546+
if (fgets((char*)userPassword, sizeof(userPassword), stdin) == NULL) {
547+
fprintf(stderr, "Getting password failed.\n");
548+
ret = WOLFSSH_USERAUTH_FAILURE;
547549
}
548550
else {
549-
printf("Password: ");
550-
fflush(stdout);
551-
ClientSetEcho(0);
552-
if (fgets((char*)userPassword, sizeof(userPassword), stdin) == NULL) {
553-
fprintf(stderr, "Getting password failed.\n");
554-
ret = WOLFSSH_USERAUTH_FAILURE;
555-
}
556-
else {
557-
char* c = strpbrk((char*)userPassword, "\r\n");
558-
if (c != NULL)
559-
*c = '\0';
560-
}
561-
passwordSz = (word32)strlen((const char*)userPassword);
562-
ClientSetEcho(1);
563-
#ifdef USE_WINDOWS_API
564-
printf("\r\n");
565-
#endif
566-
fflush(stdout);
551+
char* c = strpbrk((char*)userPassword, "\r\n");
552+
if (c != NULL)
553+
*c = '\0';
567554
}
555+
ClientSetEcho(1);
556+
#ifdef USE_WINDOWS_API
557+
printf("\r\n");
558+
#endif
559+
fflush(stdout);
568560

569561
if (ret == WOLFSSH_USERAUTH_SUCCESS) {
570562
authData->sf.password.password = userPassword;
571-
authData->sf.password.passwordSz = passwordSz;
563+
authData->sf.password.passwordSz =
564+
(word32)strlen((const char*)userPassword);
572565
}
573566
}
574567

@@ -743,7 +736,7 @@ int ClientLoadCA(WOLFSSH_CTX* ctx, const char* caCert)
743736
WFREE(der, NULL, 0);
744737
}
745738
#else
746-
(void)ctx;
739+
WOLFSSH_UNUSED(ctx);
747740
fprintf(stderr, "Support for certificates not compiled in.");
748741
ret = WS_NOT_COMPILED;
749742
#endif

‎apps/wolfssh/wolfssh.c‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -900,7 +900,6 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
900900
SOCKADDR_IN_T clientAddr;
901901
socklen_t clientAddrSz = sizeof(clientAddr);
902902
int ret = 0;
903-
const char* password = NULL;
904903
byte keepOpen = 1;
905904
#ifdef USE_WINDOWS_API
906905
byte rawMode = 0;
@@ -976,9 +975,6 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
976975
wolfSSH_SetGlobalReqCtx(ssh, &ssh); /* dummy ctx */
977976
#endif
978977

979-
if (password != NULL)
980-
wolfSSH_SetUserAuthCtx(ssh, (void*)password);
981-
982978
#ifdef WOLFSSH_AGENT
983979
if (useAgent) {
984980
WMEMSET(&agentCbCtx, 0, sizeof(agentCbCtx));

‎apps/wolfsshd/wolfsshd.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,8 @@ static byte* getBufferFromFile(const char* fileName, word32* bufSz, void* heap)
245245
word32 fileSz;
246246
word32 readSz;
247247

248+
WOLFSSH_UNUSED(heap);
249+
248250
if (fileName == NULL) return NULL;
249251

250252
if (WFOPEN(NULL, &file, fileName, "rb") != 0)
@@ -263,10 +265,9 @@ static byte* getBufferFromFile(const char* fileName, word32* bufSz, void* heap)
263265
}
264266
if (bufSz)
265267
*bufSz = readSz;
266-
WFCLOSE(NULL, file);
267268
}
269+
WFCLOSE(NULL, file);
268270

269-
(void)heap;
270271
return buf;
271272
}
272273
#endif /* NO_FILESYSTEM */

‎src/wolfsftp.c‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1997,6 +1997,12 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
19971997

19981998
WLOG(WS_LOG_SFTP, "Receiving WOLFSSH_FTP_OPEN");
19991999

2000+
#ifdef MICROCHIP_MPLAB_HARMONY
2001+
fd = WBADFILE;
2002+
#else
2003+
fd = -1;
2004+
#endif
2005+
20002006
if (sizeof(WFD) > WOLFSSH_MAX_HANDLE) {
20012007
WLOG(WS_LOG_SFTP, "Handle size is too large");
20022008
return WS_FATAL_ERROR;
@@ -2083,7 +2089,6 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
20832089
}
20842090

20852091
#ifdef MICROCHIP_MPLAB_HARMONY
2086-
fd = WBADFILE;
20872092
{
20882093
WFILE* f = &fd;
20892094
if (WFOPEN(ssh->fs, &f, dir, m) != WS_SUCCESS) {
@@ -2111,29 +2116,37 @@ int wolfSSH_SFTP_RecvOpen(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
21112116
WLOG(WS_LOG_SFTP, "Unable to store handle");
21122117
res = ier;
21132118
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
2114-
"English", NULL, &outSz) != WS_SIZE_ONLY) {
2119+
"English", NULL, &outSz) != WS_SIZE_ONLY) {
2120+
WCLOSE(ssh->fs, fd);
21152121
return WS_FATAL_ERROR;
21162122
}
21172123
ret = WS_FATAL_ERROR;
21182124
}
21192125
}
21202126
#endif
21212127

2122-
/* create packet */
2123-
out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER);
2124-
if (out == NULL) {
2125-
return WS_MEMORY_E;
2128+
if (ret == WS_SUCCESS) {
2129+
/* create packet */
2130+
out = (byte*)WMALLOC(outSz, ssh->ctx->heap, DYNTYPE_BUFFER);
2131+
if (out == NULL) {
2132+
WCLOSE(ssh->fs, fd);
2133+
return WS_MEMORY_E;
2134+
}
21262135
}
21272136
if (ret == WS_SUCCESS) {
21282137
if (SFTP_CreatePacket(ssh, WOLFSSH_FTP_HANDLE, out, outSz,
21292138
(byte*)&fd, sizeof(WFD)) != WS_SUCCESS) {
2139+
WCLOSE(ssh->fs, fd);
21302140
return WS_FATAL_ERROR;
21312141
}
21322142
}
21332143
else {
21342144
if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, res,
21352145
"English", out, &outSz) != WS_SUCCESS) {
21362146
WFREE(out, ssh->ctx->heap, DYNTYPE_BUFFER);
2147+
if (fd >= 0) {
2148+
WCLOSE(ssh->fs, fd);
2149+
}
21372150
return WS_FATAL_ERROR;
21382151
}
21392152
}

‎tests/api.c‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,15 @@ char* myoptarg = NULL;
8585

8686
#define AssertTrue(x) Assert( (x), ("%s is true", #x), (#x " => FALSE"))
8787
#define AssertFalse(x) Assert(!(x), ("%s is false", #x), (#x " => TRUE"))
88-
#define AssertNotNull(x) Assert( (x), ("%s is not null", #x), (#x " => NULL"))
88+
89+
#define AssertNotNull(x) do { \
90+
PEDANTIC_EXTENSION void* _isNotNull = (void*)(x); \
91+
Assert(_isNotNull, ("%s is not null", #x), (#x " => NULL")); \
92+
} while (0)
8993

9094
#define AssertNull(x) do { \
91-
PEDANTIC_EXTENSION void* _x = (void*)(x); \
92-
\
93-
Assert(!_x, ("%s is null", #x), (#x " => %p", _x)); \
95+
PEDANTIC_EXTENSION void* _isNull = (void*)(x); \
96+
Assert(!_isNull, ("%s is null", #x), (#x " => %p", _isNull)); \
9497
} while(0)
9598

9699
#define AssertInt(x, y, op, er) do { \
@@ -924,8 +927,17 @@ static void sftp_client_connect(WOLFSSH_CTX** ctx, WOLFSSH** ssh, int port)
924927

925928
build_addr(&clientAddr, host, port);
926929
tcp_socket(&sockFd, ((struct sockaddr_in *)&clientAddr)->sin_family);
930+
if (sockFd < 0) {
931+
wolfSSH_free(*ssh);
932+
wolfSSH_CTX_free(*ctx);
933+
*ctx = NULL;
934+
*ssh = NULL;
935+
return;
936+
}
937+
927938
ret = connect(sockFd, (const struct sockaddr *)&clientAddr, clientAddrSz);
928939
if (ret != 0){
940+
WCLOSESOCKET(sockFd);
929941
wolfSSH_free(*ssh);
930942
wolfSSH_CTX_free(*ctx);
931943
*ctx = NULL;
@@ -942,6 +954,7 @@ static void sftp_client_connect(WOLFSSH_CTX** ctx, WOLFSSH** ssh, int port)
942954
ret = wolfSSH_SFTP_connect(*ssh);
943955

944956
if (ret != WS_SUCCESS){
957+
WCLOSESOCKET(sockFd);
945958
wolfSSH_free(*ssh);
946959
wolfSSH_CTX_free(*ctx);
947960
*ctx = NULL;
@@ -1608,8 +1621,17 @@ static void keyboard_client_connect(WOLFSSH_CTX** ctx, WOLFSSH** ssh, int port)
16081621

16091622
build_addr(&clientAddr, host, port);
16101623
tcp_socket(&sockFd, ((struct sockaddr_in *)&clientAddr)->sin_family);
1624+
if (sockFd < 0) {
1625+
wolfSSH_free(*ssh);
1626+
wolfSSH_CTX_free(*ctx);
1627+
*ctx = NULL;
1628+
*ssh = NULL;
1629+
return;
1630+
}
1631+
16111632
ret = connect(sockFd, (const struct sockaddr *)&clientAddr, clientAddrSz);
16121633
if (ret != 0){
1634+
WCLOSESOCKET(sockFd);
16131635
wolfSSH_free(*ssh);
16141636
wolfSSH_CTX_free(*ctx);
16151637
*ctx = NULL;
@@ -1625,6 +1647,7 @@ static void keyboard_client_connect(WOLFSSH_CTX** ctx, WOLFSSH** ssh, int port)
16251647
ret = wolfSSH_connect(*ssh);
16261648

16271649
if (ret != WS_SUCCESS){
1650+
WCLOSESOCKET(sockFd);
16281651
wolfSSH_free(*ssh);
16291652
wolfSSH_CTX_free(*ctx);
16301653
*ctx = NULL;

‎tests/auth.c‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -443,15 +443,33 @@ static int basic_client_connect(WOLFSSH_CTX** ctx, WOLFSSH** ssh, int port)
443443
wolfSSH_CTX_free(*ctx);
444444
*ctx = NULL;
445445
*ssh = NULL;
446+
WCLOSESOCKET(sockFd);
446447
return ret;
447448
}
448449

449450
ret = wolfSSH_SetUsername(*ssh, username);
450-
if (ret == WS_SUCCESS)
451-
ret = wolfSSH_set_fd(*ssh, (int)sockFd);
451+
if (ret != WS_SUCCESS) {
452+
wolfSSH_free(*ssh);
453+
wolfSSH_CTX_free(*ctx);
454+
*ctx = NULL;
455+
*ssh = NULL;
456+
WCLOSESOCKET(sockFd);
457+
fprintf(stderr, "line= %d\n", __LINE__);
458+
return ret;
459+
}
460+
461+
ret = wolfSSH_set_fd(*ssh, (int)sockFd);
462+
if (ret != WS_SUCCESS) {
463+
fprintf(stderr, "line= %d\n", __LINE__);
464+
wolfSSH_free(*ssh);
465+
wolfSSH_CTX_free(*ctx);
466+
*ctx = NULL;
467+
*ssh = NULL;
468+
WCLOSESOCKET(sockFd);
469+
return ret;
470+
}
452471

453-
if (ret == WS_SUCCESS)
454-
ret = wolfSSH_connect(*ssh);
472+
ret = wolfSSH_connect(*ssh);
455473

456474
return ret;
457475
}

0 commit comments

Comments
 (0)