From f5c253e43c6b950de6f140c124b6d23e9a57e178 Mon Sep 17 00:00:00 2001 From: Maks Mishin Date: Thu, 14 Dec 2023 00:10:19 +0300 Subject: [PATCH 1/4] Add error handling for descriptor leak(CWE-403) --- src/iperf_server_api.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/iperf_server_api.c b/src/iperf_server_api.c index 77e9c355c..1781e5af8 100644 --- a/src/iperf_server_api.c +++ b/src/iperf_server_api.c @@ -137,6 +137,7 @@ int iperf_accept(struct iperf_test *test) { int s; + int ret = -1; signed char rbuf = ACCESS_DENIED; socklen_t len; struct sockaddr_storage addr; @@ -144,7 +145,7 @@ iperf_accept(struct iperf_test *test) len = sizeof(addr); if ((s = accept(test->listener, (struct sockaddr *) &addr, &len)) < 0) { i_errno = IEACCEPT; - return -1; + return ret; } if (test->ctrl_sck == -1) { @@ -154,7 +155,7 @@ iperf_accept(struct iperf_test *test) int flag = 1; if (setsockopt(test->ctrl_sck, IPPROTO_TCP, TCP_NODELAY, (char *) &flag, sizeof(int))) { i_errno = IESETNODELAY; - return -1; + goto error_handling; } #if defined(HAVE_TCP_USER_TIMEOUT) @@ -162,7 +163,7 @@ iperf_accept(struct iperf_test *test) if ((opt = test->settings->snd_timeout)) { if (setsockopt(s, IPPROTO_TCP, TCP_USER_TIMEOUT, &opt, sizeof(opt)) < 0) { i_errno = IESETUSERTIMEOUT; - return -1; + goto error_handling; } } #endif /* HAVE_TCP_USER_TIMEOUT */ @@ -174,18 +175,18 @@ iperf_accept(struct iperf_test *test) * (i.e. timed out). */ i_errno = IERECVCOOKIE; - return -1; + goto error_handling; } - FD_SET(test->ctrl_sck, &test->read_set); - if (test->ctrl_sck > test->max_fd) test->max_fd = test->ctrl_sck; + FD_SET(test->ctrl_sck, &test->read_set); + if (test->ctrl_sck > test->max_fd) test->max_fd = test->ctrl_sck; - if (iperf_set_send_state(test, PARAM_EXCHANGE) != 0) - return -1; - if (iperf_exchange_parameters(test) < 0) - return -1; - if (test->server_affinity != -1) - if (iperf_setaffinity(test, test->server_affinity) != 0) - return -1; + if (iperf_set_send_state(test, PARAM_EXCHANGE) != 0) + goto error_handling; + if (iperf_exchange_parameters(test) < 0) + goto error_handling; + if (test->server_affinity != -1) + if (iperf_setaffinity(test, test->server_affinity) != 0) + goto error_handling; if (test->on_connect) test->on_connect(test); } else { @@ -204,8 +205,10 @@ iperf_accept(struct iperf_test *test) } close(s); } - return 0; + error_handling: + close(s); + return ret; } From 60b0c18db0a84c331444ddc70619f9c4c3f66cc1 Mon Sep 17 00:00:00 2001 From: rabijl Date: Tue, 19 Mar 2024 19:39:28 +0200 Subject: [PATCH 2/4] iperf_api: memset entire malloc in the function iperf_new_test the bitrate_limit_intervals_traffic_bytes array was only memset for the size of the sizeof return type, instead of the entire array. --- src/iperf_api.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/iperf_api.c b/src/iperf_api.c index 4765d4e97..7887235ca 100644 --- a/src/iperf_api.c +++ b/src/iperf_api.c @@ -2878,7 +2878,7 @@ iperf_new_test() i_errno = IENEWTEST; return NULL; } - memset(test->bitrate_limit_intervals_traffic_bytes, 0, sizeof(sizeof(iperf_size_t) * MAX_INTERVAL)); + memset(test->bitrate_limit_intervals_traffic_bytes, 0, sizeof(iperf_size_t) * MAX_INTERVAL); /* By default all output goes to stdout */ test->outfile = stdout; From 356f01f91485937173e5557c61d5a41b845a1943 Mon Sep 17 00:00:00 2001 From: Matthew Cather <14895427+MattCatz@users.noreply.github.com> Date: Fri, 10 May 2024 16:27:29 -0500 Subject: [PATCH 3/4] Initialize cookie buffer `Nread` reads up *to* N bytes from the socket. Since we only check that we read more than 0 bytes, it's possible for the cookie buffer only be partially initialized (and may not contain a valid null terminated string). Initializing the buffer to 0 fixes this. Also swap `strcmp` with `strncmp` since we know know exactly how long a cookie should be. This will help prevent any buffer overflows if the length of the cookie ever changes for some reason. --- src/iperf_tcp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/iperf_tcp.c b/src/iperf_tcp.c index 71c40a6ea..e025515ab 100644 --- a/src/iperf_tcp.c +++ b/src/iperf_tcp.c @@ -117,7 +117,7 @@ iperf_tcp_accept(struct iperf_test * test) { int s; signed char rbuf = ACCESS_DENIED; - char cookie[COOKIE_SIZE]; + char cookie[COOKIE_SIZE] = {0}; socklen_t len; struct sockaddr_storage addr; @@ -149,7 +149,7 @@ iperf_tcp_accept(struct iperf_test * test) return -1; } - if (strcmp(test->cookie, cookie) != 0) { + if (strncmp(test->cookie, cookie, COOKIE_SIZE) != 0) { if (Nwrite(s, (char*) &rbuf, sizeof(rbuf), Ptcp) < 0) { iperf_err(test, "failed to send access denied from busy server to new connecting client, errno = %d\n", errno); } From 6f1e96ffe230b38efcf92856bfd9535279c93586 Mon Sep 17 00:00:00 2001 From: tqfx Date: Tue, 23 Apr 2024 12:29:16 +0800 Subject: [PATCH 4/4] Fix clang compilation failure due to patch d80b914141582dc0c0cf3f71cc5799061074be66 --- src/iperf_pthread.c | 27 ++++++++++++++------------- src/iperf_pthread.h | 4 ++-- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/iperf_pthread.c b/src/iperf_pthread.c index ea4918bcd..9798a4112 100644 --- a/src/iperf_pthread.c +++ b/src/iperf_pthread.c @@ -6,21 +6,12 @@ * as Android NDK does not support `pthread_cancel()`. */ +#include #include #include "iperf_pthread.h" -int pthread_setcanceltype(int type, int *oldtype) { return 0; } -int pthread_setcancelstate(int state, int *oldstate) { return 0; } -int pthread_cancel(pthread_t thread_id) { - int status; - if ((status = iperf_set_thread_exit_handler()) == 0) { - status = pthread_kill(thread_id, SIGUSR1); - } - return status; -} - void iperf_thread_exit_handler(int sig) -{ +{ pthread_exit(0); } @@ -28,13 +19,23 @@ int iperf_set_thread_exit_handler() { int rc; struct sigaction actions; - memset(&actions, 0, sizeof(actions)); + memset(&actions, 0, sizeof(actions)); sigemptyset(&actions.sa_mask); - actions.sa_flags = 0; + actions.sa_flags = 0; actions.sa_handler = iperf_thread_exit_handler; rc = sigaction(SIGUSR1, &actions, NULL); return rc; } +int pthread_setcanceltype(int type, int *oldtype) { return 0; } +int pthread_setcancelstate(int state, int *oldstate) { return 0; } +int pthread_cancel(pthread_t thread_id) { + int status; + if ((status = iperf_set_thread_exit_handler()) == 0) { + status = pthread_kill(thread_id, SIGUSR1); + } + return status; +} + #endif // defined(HAVE_PTHREAD) && defined(__ANDROID__) diff --git a/src/iperf_pthread.h b/src/iperf_pthread.h index 44828d6a9..9fe3db8a2 100644 --- a/src/iperf_pthread.h +++ b/src/iperf_pthread.h @@ -10,7 +10,7 @@ */ #define PTHREAD_CANCEL_ASYNCHRONOUS 0 -#define PTHREAD_CANCEL_ENABLE NULL +#define PTHREAD_CANCEL_ENABLE 0 int pthread_setcanceltype(int type, int *oldtype); int pthread_setcancelstate(int state, int *oldstate); @@ -18,4 +18,4 @@ int pthread_cancel(pthread_t thread_id); #endif // defined(__ANDROID__) -#endif // defined(HAVE_PTHREAD) \ No newline at end of file +#endif // defined(HAVE_PTHREAD)