Skip to content

Commit fc68795

Browse files
committed
Coverity: Resource leak
1. Fix some resource leaks during error conditions where a socket or a file descriptor doesn't get closed in all error cases. 2. In wolfSSH_SFTP_RecvOpen(), initialize the file descriptor. 3. For 572902, the error case resource leaks are fixed. There's still an issue to resolve for storing the FD for use later. Fixes CIDs: 572856 572902* 573012 573019 573021 573076
1 parent 0724553 commit fc68795

4 files changed

Lines changed: 64 additions & 12 deletions

File tree

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: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -927,8 +927,17 @@ static void sftp_client_connect(WOLFSSH_CTX** ctx, WOLFSSH** ssh, int port)
927927

928928
build_addr(&clientAddr, host, port);
929929
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+
930938
ret = connect(sockFd, (const struct sockaddr *)&clientAddr, clientAddrSz);
931939
if (ret != 0){
940+
WCLOSESOCKET(sockFd);
932941
wolfSSH_free(*ssh);
933942
wolfSSH_CTX_free(*ctx);
934943
*ctx = NULL;
@@ -945,6 +954,7 @@ static void sftp_client_connect(WOLFSSH_CTX** ctx, WOLFSSH** ssh, int port)
945954
ret = wolfSSH_SFTP_connect(*ssh);
946955

947956
if (ret != WS_SUCCESS){
957+
WCLOSESOCKET(sockFd);
948958
wolfSSH_free(*ssh);
949959
wolfSSH_CTX_free(*ctx);
950960
*ctx = NULL;
@@ -1611,8 +1621,17 @@ static void keyboard_client_connect(WOLFSSH_CTX** ctx, WOLFSSH** ssh, int port)
16111621

16121622
build_addr(&clientAddr, host, port);
16131623
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+
16141632
ret = connect(sockFd, (const struct sockaddr *)&clientAddr, clientAddrSz);
16151633
if (ret != 0){
1634+
WCLOSESOCKET(sockFd);
16161635
wolfSSH_free(*ssh);
16171636
wolfSSH_CTX_free(*ctx);
16181637
*ctx = NULL;
@@ -1628,6 +1647,7 @@ static void keyboard_client_connect(WOLFSSH_CTX** ctx, WOLFSSH** ssh, int port)
16281647
ret = wolfSSH_connect(*ssh);
16291648

16301649
if (ret != WS_SUCCESS){
1650+
WCLOSESOCKET(sockFd);
16311651
wolfSSH_free(*ssh);
16321652
wolfSSH_CTX_free(*ctx);
16331653
*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)