From 3f66f604df7f1038a49108c48612c2f4fe71331f Mon Sep 17 00:00:00 2001 From: Sarah Larsen Date: Fri, 15 Nov 2024 23:23:05 +0000 Subject: [PATCH] Add a variant of cJSON_GetObjectItem that does type-checking. This avoids a potential server crash with malformed iperf3 parameter sets. (CVE-2024-53580) Vulnerability report submitted by Leonid Krolle Bi.Zone. Original version of fix by @dopheide-esnet. --- src/iperf_api.c | 98 +++++++++++++++++++++++------------------------ src/iperf_error.c | 6 +-- src/iperf_util.c | 38 +++++++++++++++++- src/iperf_util.h | 1 + 4 files changed, 90 insertions(+), 53 deletions(-) diff --git a/src/iperf_util.c b/src/iperf_util.c index 22ff43a..bf2c408 100644 --- a/src/iperf_util.c +++ b/src/iperf_util.c @@ -378,6 +378,42 @@ iperf_json_printf(const char *format, ...) return o; } +/********************** cJSON GetObjectItem w/ Type Helper ********************/ +cJSON * iperf_cJSON_GetObjectItemType(cJSON * j, char * item_string, int expected_type){ + cJSON *j_p; + if((j_p = cJSON_GetObjectItem(j, item_string)) != NULL) + switch(expected_type){ + case cJSON_True: + if(cJSON_IsBool(j_p)) + return j_p; + else + iperf_err(NULL, "iperf_cJSON_GetObjectItemType mismatch %s", item_string); + break; + case cJSON_String: + if(cJSON_IsString(j_p)) + return j_p; + else + iperf_err(NULL, "iperf_cJSON_GetObjectItemType mismatch %s", item_string); + break; + case cJSON_Number: + if(cJSON_IsNumber(j_p)) + return j_p; + else + iperf_err(NULL, "iperf_cJSON_GetObjectItemType mismatch %s", item_string); + break; + case cJSON_Array: + if(cJSON_IsArray(j_p)) + return j_p; + else + iperf_err(NULL, "iperf_cJSON_GetObjectItemType mismatch %s", item_string); + break; + default: + iperf_err(NULL, "unsupported type"); + } + + return NULL; +} + /* Debugging routine to dump out an fd_set. */ void iperf_dump_fdset(FILE *fp, char *str, int nfds, fd_set *fds) diff --git a/src/iperf_util.h b/src/iperf_util.h index ee1d58c..0a33214 100644 --- a/src/iperf_util.h +++ b/src/iperf_util.h @@ -51,6 +51,7 @@ const char* get_system_info(void); const char* get_optional_features(void); cJSON* iperf_json_printf(const char *format, ...); +cJSON * iperf_cJSON_GetObjectItemType(cJSON * j_p, char * item_string, int expected_type); void iperf_dump_fdset(FILE *fp, char *str, int nfds, fd_set *fds); diff --git a/src/iperf_error.c b/src/iperf_error.c index 945984e..f90d03f 100644 --- a/src/iperf_error.c +++ b/src/iperf_error.c @@ -45,7 +45,7 @@ iperf_err(struct iperf_test *test, const char *format, ...) if (test != NULL && test->json_output && test->json_top != NULL) cJSON_AddStringToObject(test->json_top, "error", str); else - if (test && test->outfile && test->outfile != stdout) { + if (test != NULL && test->outfile != NULL && test->outfile != stdout) { fprintf(test->outfile, "iperf3: %s\n", str); } else { diff --git a/src/iperf_api.c b/src/iperf_api.c index 549ffcc..34b90c2 100755 --- a/src/iperf_api.c +++ b/src/iperf_api.c @@ -1547,58 +1547,58 @@ get_parameters(struct iperf_test *test) printf("get_parameters:\n%s\n", cJSON_Print(j)); } - if ((j_p = cJSON_GetObjectItem(j, "tcp")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "tcp", cJSON_True)) != NULL) set_protocol(test, Ptcp); - if ((j_p = cJSON_GetObjectItem(j, "udp")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "udp", cJSON_True)) != NULL) set_protocol(test, Pudp); - if ((j_p = cJSON_GetObjectItem(j, "sctp")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "sctp", cJSON_True)) != NULL) set_protocol(test, Psctp); - if ((j_p = cJSON_GetObjectItem(j, "omit")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "omit", cJSON_Number)) != NULL) test->omit = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "server_affinity")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "server_affinity", cJSON_Number)) != NULL) test->server_affinity = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "time")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "time", cJSON_Number)) != NULL) test->duration = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "num")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "num", cJSON_Number)) != NULL) test->settings->bytes = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "blockcount")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "blockcount", cJSON_Number)) != NULL) test->settings->blocks = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "MSS")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "MSS", cJSON_Number)) != NULL) test->settings->mss = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "nodelay")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "nodelay", cJSON_True)) != NULL) test->no_delay = 1; - if ((j_p = cJSON_GetObjectItem(j, "parallel")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "parallel", cJSON_Number)) != NULL) test->num_streams = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "reverse")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "reverse", cJSON_True)) != NULL) iperf_set_test_reverse(test, 1); - if ((j_p = cJSON_GetObjectItem(j, "window")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "window", cJSON_Number)) != NULL) test->settings->socket_bufsize = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "len")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "len", cJSON_Number)) != NULL) test->settings->blksize = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "bandwidth")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "bandwidth", cJSON_Number)) != NULL) test->settings->rate = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "fqrate")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "fqrate", cJSON_Number)) != NULL) test->settings->fqrate = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "pacing_timer")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "pacing_timer", cJSON_Number)) != NULL) test->settings->pacing_timer = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "burst")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "burst", cJSON_Number)) != NULL) test->settings->burst = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "TOS")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "TOS", cJSON_Number)) != NULL) test->settings->tos = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "flowlabel")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "flowlabel", cJSON_Number)) != NULL) test->settings->flowlabel = j_p->valueint; - if ((j_p = cJSON_GetObjectItem(j, "title")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "title", cJSON_String)) != NULL) test->title = strdup(j_p->valuestring); - if ((j_p = cJSON_GetObjectItem(j, "congestion")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "congestion", cJSON_String)) != NULL) test->congestion = strdup(j_p->valuestring); - if ((j_p = cJSON_GetObjectItem(j, "congestion_used")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "congestion_used", cJSON_String)) != NULL) test->congestion_used = strdup(j_p->valuestring); - if ((j_p = cJSON_GetObjectItem(j, "get_server_output")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "get_server_output", cJSON_Number)) != NULL) iperf_set_test_get_server_output(test, 1); - if ((j_p = cJSON_GetObjectItem(j, "udp_counters_64bit")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "udp_counters_64bit", cJSON_Number)) != NULL) iperf_set_test_udp_counters_64bit(test, 1); #if defined(HAVE_SSL) - if ((j_p = cJSON_GetObjectItem(j, "authtoken")) != NULL) + if ((j_p = iperf_cJSON_GetObjectItemType(j, "authtoken", cJSON_String)) != NULL) test->settings->authtoken = strdup(j_p->valuestring); #endif //HAVE_SSL if (test->sender && test->protocol->id == Ptcp && has_tcpinfo_retransmits()) @@ -1745,10 +1745,10 @@ get_results(struct iperf_test *test) i_errno = IERECVRESULTS; r = -1; } else { - j_cpu_util_total = cJSON_GetObjectItem(j, "cpu_util_total"); - j_cpu_util_user = cJSON_GetObjectItem(j, "cpu_util_user"); - j_cpu_util_system = cJSON_GetObjectItem(j, "cpu_util_system"); - j_sender_has_retransmits = cJSON_GetObjectItem(j, "sender_has_retransmits"); + j_cpu_util_total = iperf_cJSON_GetObjectItemType(j, "cpu_util_total", cJSON_Number); + j_cpu_util_user = iperf_cJSON_GetObjectItemType(j, "cpu_util_user", cJSON_Number); + j_cpu_util_system = iperf_cJSON_GetObjectItemType(j, "cpu_util_system", cJSON_Number); + j_sender_has_retransmits = iperf_cJSON_GetObjectItemType(j, "sender_has_retransmits", cJSON_Number); if (j_cpu_util_total == NULL || j_cpu_util_user == NULL || j_cpu_util_system == NULL || j_sender_has_retransmits == NULL) { i_errno = IERECVRESULTS; r = -1; @@ -1763,7 +1763,7 @@ get_results(struct iperf_test *test) result_has_retransmits = j_sender_has_retransmits->valueint; if (! test->sender) test->sender_has_retransmits = result_has_retransmits; - j_streams = cJSON_GetObjectItem(j, "streams"); + j_streams = iperf_cJSON_GetObjectItemType(j, "streams", cJSON_Array); if (j_streams == NULL) { i_errno = IERECVRESULTS; r = -1; @@ -1775,14 +1775,14 @@ get_results(struct iperf_test *test) i_errno = IERECVRESULTS; r = -1; } else { - j_id = cJSON_GetObjectItem(j_stream, "id"); - j_bytes = cJSON_GetObjectItem(j_stream, "bytes"); - j_retransmits = cJSON_GetObjectItem(j_stream, "retransmits"); - j_jitter = cJSON_GetObjectItem(j_stream, "jitter"); - j_errors = cJSON_GetObjectItem(j_stream, "errors"); - j_packets = cJSON_GetObjectItem(j_stream, "packets"); - j_start_time = cJSON_GetObjectItem(j_stream, "start_time"); - j_end_time = cJSON_GetObjectItem(j_stream, "end_time"); + j_id = iperf_cJSON_GetObjectItemType(j_stream, "id", cJSON_Number); + j_bytes = iperf_cJSON_GetObjectItemType(j_stream, "bytes", cJSON_Number); + j_retransmits = iperf_cJSON_GetObjectItemType(j_stream, "retransmits", cJSON_Number); + j_jitter = iperf_cJSON_GetObjectItemType(j_stream, "jitter", cJSON_Number); + j_errors = iperf_cJSON_GetObjectItemType(j_stream, "errors", cJSON_Number); + j_packets = iperf_cJSON_GetObjectItemType(j_stream, "packets", cJSON_Number); + j_start_time = iperf_cJSON_GetObjectItemType(j_stream, "start_time", cJSON_Number); + j_end_time = iperf_cJSON_GetObjectItemType(j_stream, "end_time", cJSON_Number); if (j_id == NULL || j_bytes == NULL || j_retransmits == NULL || j_jitter == NULL || j_errors == NULL || j_packets == NULL) { i_errno = IERECVRESULTS; r = -1; @@ -1846,7 +1846,7 @@ get_results(struct iperf_test *test) } else { /* No JSON, look for textual output. Make a copy of the text for later. */ - j_server_output = cJSON_GetObjectItem(j, "server_output_text"); + j_server_output = iperf_cJSON_GetObjectItemType(j, "server_output_text", cJSON_String); if (j_server_output != NULL) { test->server_output_text = strdup(j_server_output->valuestring); } @@ -1855,7 +1855,7 @@ get_results(struct iperf_test *test) } } - j_remote_congestion_used = cJSON_GetObjectItem(j, "congestion_used"); + j_remote_congestion_used = iperf_cJSON_GetObjectItemType(j, "congestion_used", cJSON_String); if (j_remote_congestion_used != NULL) { test->remote_congestion_used = strdup(j_remote_congestion_used->valuestring); }