Skip to content

Commit 3f66f60

Browse files
swlarsbmah888
swlars
authored and
bmah888
committed
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.
1 parent d595ee6 commit 3f66f60

File tree

4 files changed

+90
-53
lines changed

4 files changed

+90
-53
lines changed

‎src/iperf_api.c

+49-49
Original file line numberDiff line numberDiff line change
@@ -2347,72 +2347,72 @@ get_parameters(struct iperf_test *test)
23472347
cJSON_free(str);
23482348
}
23492349

2350-
if ((j_p = cJSON_GetObjectItem(j, "tcp")) != NULL)
2350+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "tcp", cJSON_True)) != NULL)
23512351
set_protocol(test, Ptcp);
2352-
if ((j_p = cJSON_GetObjectItem(j, "udp")) != NULL)
2352+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "udp", cJSON_True)) != NULL)
23532353
set_protocol(test, Pudp);
2354-
if ((j_p = cJSON_GetObjectItem(j, "sctp")) != NULL)
2354+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "sctp", cJSON_True)) != NULL)
23552355
set_protocol(test, Psctp);
2356-
if ((j_p = cJSON_GetObjectItem(j, "omit")) != NULL)
2356+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "omit", cJSON_Number)) != NULL)
23572357
test->omit = j_p->valueint;
2358-
if ((j_p = cJSON_GetObjectItem(j, "server_affinity")) != NULL)
2358+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "server_affinity", cJSON_Number)) != NULL)
23592359
test->server_affinity = j_p->valueint;
2360-
if ((j_p = cJSON_GetObjectItem(j, "time")) != NULL)
2360+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "time", cJSON_Number)) != NULL)
23612361
test->duration = j_p->valueint;
23622362
test->settings->bytes = 0;
2363-
if ((j_p = cJSON_GetObjectItem(j, "num")) != NULL)
2363+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "num", cJSON_Number)) != NULL)
23642364
test->settings->bytes = j_p->valueint;
23652365
test->settings->blocks = 0;
2366-
if ((j_p = cJSON_GetObjectItem(j, "blockcount")) != NULL)
2366+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "blockcount", cJSON_Number)) != NULL)
23672367
test->settings->blocks = j_p->valueint;
2368-
if ((j_p = cJSON_GetObjectItem(j, "MSS")) != NULL)
2368+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "MSS", cJSON_Number)) != NULL)
23692369
test->settings->mss = j_p->valueint;
2370-
if ((j_p = cJSON_GetObjectItem(j, "nodelay")) != NULL)
2370+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "nodelay", cJSON_True)) != NULL)
23712371
test->no_delay = 1;
2372-
if ((j_p = cJSON_GetObjectItem(j, "parallel")) != NULL)
2372+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "parallel", cJSON_Number)) != NULL)
23732373
test->num_streams = j_p->valueint;
2374-
if ((j_p = cJSON_GetObjectItem(j, "reverse")) != NULL)
2374+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "reverse", cJSON_True)) != NULL)
23752375
iperf_set_test_reverse(test, 1);
2376-
if ((j_p = cJSON_GetObjectItem(j, "bidirectional")) != NULL)
2376+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "bidirectional", cJSON_True)) != NULL)
23772377
iperf_set_test_bidirectional(test, 1);
2378-
if ((j_p = cJSON_GetObjectItem(j, "window")) != NULL)
2378+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "window", cJSON_Number)) != NULL)
23792379
test->settings->socket_bufsize = j_p->valueint;
2380-
if ((j_p = cJSON_GetObjectItem(j, "len")) != NULL)
2380+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "len", cJSON_Number)) != NULL)
23812381
test->settings->blksize = j_p->valueint;
2382-
if ((j_p = cJSON_GetObjectItem(j, "bandwidth")) != NULL)
2382+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "bandwidth", cJSON_Number)) != NULL)
23832383
test->settings->rate = j_p->valueint;
2384-
if ((j_p = cJSON_GetObjectItem(j, "fqrate")) != NULL)
2384+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "fqrate", cJSON_Number)) != NULL)
23852385
test->settings->fqrate = j_p->valueint;
2386-
if ((j_p = cJSON_GetObjectItem(j, "pacing_timer")) != NULL)
2386+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "pacing_timer", cJSON_Number)) != NULL)
23872387
test->settings->pacing_timer = j_p->valueint;
2388-
if ((j_p = cJSON_GetObjectItem(j, "burst")) != NULL)
2388+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "burst", cJSON_Number)) != NULL)
23892389
test->settings->burst = j_p->valueint;
2390-
if ((j_p = cJSON_GetObjectItem(j, "TOS")) != NULL)
2390+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "TOS", cJSON_Number)) != NULL)
23912391
test->settings->tos = j_p->valueint;
2392-
if ((j_p = cJSON_GetObjectItem(j, "flowlabel")) != NULL)
2392+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "flowlabel", cJSON_Number)) != NULL)
23932393
test->settings->flowlabel = j_p->valueint;
2394-
if ((j_p = cJSON_GetObjectItem(j, "title")) != NULL)
2394+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "title", cJSON_String)) != NULL)
23952395
test->title = strdup(j_p->valuestring);
2396-
if ((j_p = cJSON_GetObjectItem(j, "extra_data")) != NULL)
2396+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "extra_data", cJSON_String)) != NULL)
23972397
test->extra_data = strdup(j_p->valuestring);
2398-
if ((j_p = cJSON_GetObjectItem(j, "congestion")) != NULL)
2398+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "congestion", cJSON_String)) != NULL)
23992399
test->congestion = strdup(j_p->valuestring);
2400-
if ((j_p = cJSON_GetObjectItem(j, "congestion_used")) != NULL)
2400+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "congestion_used", cJSON_String)) != NULL)
24012401
test->congestion_used = strdup(j_p->valuestring);
2402-
if ((j_p = cJSON_GetObjectItem(j, "get_server_output")) != NULL)
2402+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "get_server_output", cJSON_Number)) != NULL)
24032403
iperf_set_test_get_server_output(test, 1);
2404-
if ((j_p = cJSON_GetObjectItem(j, "udp_counters_64bit")) != NULL)
2404+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "udp_counters_64bit", cJSON_Number)) != NULL)
24052405
iperf_set_test_udp_counters_64bit(test, 1);
2406-
if ((j_p = cJSON_GetObjectItem(j, "repeating_payload")) != NULL)
2406+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "repeating_payload", cJSON_Number)) != NULL)
24072407
test->repeating_payload = 1;
2408-
if ((j_p = cJSON_GetObjectItem(j, "zerocopy")) != NULL)
2408+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "zerocopy", cJSON_Number)) != NULL)
24092409
test->zerocopy = j_p->valueint;
24102410
#if defined(HAVE_DONT_FRAGMENT)
2411-
if ((j_p = cJSON_GetObjectItem(j, "dont_fragment")) != NULL)
2411+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "dont_fragment", cJSON_Number)) != NULL)
24122412
test->settings->dont_fragment = j_p->valueint;
24132413
#endif /* HAVE_DONT_FRAGMENT */
24142414
#if defined(HAVE_SSL)
2415-
if ((j_p = cJSON_GetObjectItem(j, "authtoken")) != NULL)
2415+
if ((j_p = iperf_cJSON_GetObjectItemType(j, "authtoken", cJSON_String)) != NULL)
24162416
test->settings->authtoken = strdup(j_p->valuestring);
24172417
#endif //HAVE_SSL
24182418
if (test->mode && test->protocol->id == Ptcp && has_tcpinfo_retransmits())
@@ -2571,10 +2571,10 @@ get_results(struct iperf_test *test)
25712571
i_errno = IERECVRESULTS;
25722572
r = -1;
25732573
} else {
2574-
j_cpu_util_total = cJSON_GetObjectItem(j, "cpu_util_total");
2575-
j_cpu_util_user = cJSON_GetObjectItem(j, "cpu_util_user");
2576-
j_cpu_util_system = cJSON_GetObjectItem(j, "cpu_util_system");
2577-
j_sender_has_retransmits = cJSON_GetObjectItem(j, "sender_has_retransmits");
2574+
j_cpu_util_total = iperf_cJSON_GetObjectItemType(j, "cpu_util_total", cJSON_Number);
2575+
j_cpu_util_user = iperf_cJSON_GetObjectItemType(j, "cpu_util_user", cJSON_Number);
2576+
j_cpu_util_system = iperf_cJSON_GetObjectItemType(j, "cpu_util_system", cJSON_Number);
2577+
j_sender_has_retransmits = iperf_cJSON_GetObjectItemType(j, "sender_has_retransmits", cJSON_Number);
25782578
if (j_cpu_util_total == NULL || j_cpu_util_user == NULL || j_cpu_util_system == NULL || j_sender_has_retransmits == NULL) {
25792579
i_errno = IERECVRESULTS;
25802580
r = -1;
@@ -2596,7 +2596,7 @@ get_results(struct iperf_test *test)
25962596
else if ( test->mode == BIDIRECTIONAL )
25972597
test->other_side_has_retransmits = result_has_retransmits;
25982598

2599-
j_streams = cJSON_GetObjectItem(j, "streams");
2599+
j_streams = iperf_cJSON_GetObjectItemType(j, "streams", cJSON_Array);
26002600
if (j_streams == NULL) {
26012601
i_errno = IERECVRESULTS;
26022602
r = -1;
@@ -2608,16 +2608,16 @@ get_results(struct iperf_test *test)
26082608
i_errno = IERECVRESULTS;
26092609
r = -1;
26102610
} else {
2611-
j_id = cJSON_GetObjectItem(j_stream, "id");
2612-
j_bytes = cJSON_GetObjectItem(j_stream, "bytes");
2613-
j_retransmits = cJSON_GetObjectItem(j_stream, "retransmits");
2614-
j_jitter = cJSON_GetObjectItem(j_stream, "jitter");
2615-
j_errors = cJSON_GetObjectItem(j_stream, "errors");
2616-
j_omitted_errors = cJSON_GetObjectItem(j_stream, "omitted_errors");
2617-
j_packets = cJSON_GetObjectItem(j_stream, "packets");
2618-
j_omitted_packets = cJSON_GetObjectItem(j_stream, "omitted_packets");
2619-
j_start_time = cJSON_GetObjectItem(j_stream, "start_time");
2620-
j_end_time = cJSON_GetObjectItem(j_stream, "end_time");
2611+
j_id = iperf_cJSON_GetObjectItemType(j_stream, "id", cJSON_Number);
2612+
j_bytes = iperf_cJSON_GetObjectItemType(j_stream, "bytes", cJSON_Number);
2613+
j_retransmits = iperf_cJSON_GetObjectItemType(j_stream, "retransmits", cJSON_Number);
2614+
j_jitter = iperf_cJSON_GetObjectItemType(j_stream, "jitter", cJSON_Number);
2615+
j_errors = iperf_cJSON_GetObjectItemType(j_stream, "errors", cJSON_Number);
2616+
j_omitted_errors = iperf_cJSON_GetObjectItemType(j_stream, "omitted_errors", cJSON_Number);
2617+
j_packets = iperf_cJSON_GetObjectItemType(j_stream, "packets", cJSON_Number);
2618+
j_omitted_packets = iperf_cJSON_GetObjectItemType(j_stream, "omitted_packets", cJSON_Number);
2619+
j_start_time = iperf_cJSON_GetObjectItemType(j_stream, "start_time", cJSON_Number);
2620+
j_end_time = iperf_cJSON_GetObjectItemType(j_stream, "end_time", cJSON_Number);
26212621
if (j_id == NULL || j_bytes == NULL || j_retransmits == NULL || j_jitter == NULL || j_errors == NULL || j_packets == NULL) {
26222622
i_errno = IERECVRESULTS;
26232623
r = -1;
@@ -2706,7 +2706,7 @@ get_results(struct iperf_test *test)
27062706
}
27072707
else {
27082708
/* No JSON, look for textual output. Make a copy of the text for later. */
2709-
j_server_output = cJSON_GetObjectItem(j, "server_output_text");
2709+
j_server_output = iperf_cJSON_GetObjectItemType(j, "server_output_text", cJSON_String);
27102710
if (j_server_output != NULL) {
27112711
test->server_output_text = strdup(j_server_output->valuestring);
27122712
}
@@ -2715,7 +2715,7 @@ get_results(struct iperf_test *test)
27152715
}
27162716
}
27172717

2718-
j_remote_congestion_used = cJSON_GetObjectItem(j, "congestion_used");
2718+
j_remote_congestion_used = iperf_cJSON_GetObjectItemType(j, "congestion_used", cJSON_String);
27192719
if (j_remote_congestion_used != NULL) {
27202720
test->remote_congestion_used = strdup(j_remote_congestion_used->valuestring);
27212721
}
@@ -4960,7 +4960,7 @@ iperf_json_finish(struct iperf_test *test)
49604960

49614961
/* --json-stream, so we print various individual objects */
49624962
if (test->json_stream) {
4963-
cJSON *error = cJSON_GetObjectItem(test->json_top, "error");
4963+
cJSON *error = iperf_cJSON_GetObjectItemType(test->json_top, "error", cJSON_String);
49644964
if (error) {
49654965
JSONStream_Output(test, "error", error);
49664966
}

‎src/iperf_error.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,11 @@ iperf_err(struct iperf_test *test, const char *format, ...)
6060
if (test != NULL && test->json_output && test->json_top != NULL)
6161
cJSON_AddStringToObject(test->json_top, "error", str);
6262
else {
63-
if (pthread_mutex_lock(&(test->print_mutex)) != 0) {
63+
if (test != NULL && pthread_mutex_lock(&(test->print_mutex)) != 0) {
6464
perror("iperf_err: pthread_mutex_lock");
6565
}
6666

67-
if (test && test->outfile && test->outfile != stdout) {
67+
if (test != NULL && test->outfile != NULL && test->outfile != stdout) {
6868
if (ct) {
6969
fprintf(test->outfile, "%s", ct);
7070
}
@@ -77,7 +77,7 @@ iperf_err(struct iperf_test *test, const char *format, ...)
7777
fprintf(stderr, "iperf3: %s\n", str);
7878
}
7979

80-
if (pthread_mutex_unlock(&(test->print_mutex)) != 0) {
80+
if (test != NULL && pthread_mutex_unlock(&(test->print_mutex)) != 0) {
8181
perror("iperf_err: pthread_mutex_unlock");
8282
}
8383

‎src/iperf_util.c

+37-1
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,42 @@ iperf_json_printf(const char *format, ...)
430430
return o;
431431
}
432432

433+
/********************** cJSON GetObjectItem w/ Type Helper ********************/
434+
cJSON * iperf_cJSON_GetObjectItemType(cJSON * j, char * item_string, int expected_type){
435+
cJSON *j_p;
436+
if((j_p = cJSON_GetObjectItem(j, item_string)) != NULL)
437+
switch(expected_type){
438+
case cJSON_True:
439+
if(cJSON_IsBool(j_p))
440+
return j_p;
441+
else
442+
iperf_err(NULL, "iperf_cJSON_GetObjectItemType mismatch %s", item_string);
443+
break;
444+
case cJSON_String:
445+
if(cJSON_IsString(j_p))
446+
return j_p;
447+
else
448+
iperf_err(NULL, "iperf_cJSON_GetObjectItemType mismatch %s", item_string);
449+
break;
450+
case cJSON_Number:
451+
if(cJSON_IsNumber(j_p))
452+
return j_p;
453+
else
454+
iperf_err(NULL, "iperf_cJSON_GetObjectItemType mismatch %s", item_string);
455+
break;
456+
case cJSON_Array:
457+
if(cJSON_IsArray(j_p))
458+
return j_p;
459+
else
460+
iperf_err(NULL, "iperf_cJSON_GetObjectItemType mismatch %s", item_string);
461+
break;
462+
default:
463+
iperf_err(NULL, "unsupported type");
464+
}
465+
466+
return NULL;
467+
}
468+
433469
/* Debugging routine to dump out an fd_set. */
434470
void
435471
iperf_dump_fdset(FILE *fp, const char *str, int nfds, fd_set *fds)
@@ -621,4 +657,4 @@ state_to_text(signed char state)
621657
}
622658

623659
return txt;
624-
}
660+
}

‎src/iperf_util.h

+1
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ const char* get_system_info(void);
5353
const char* get_optional_features(void);
5454

5555
cJSON* iperf_json_printf(const char *format, ...);
56+
cJSON * iperf_cJSON_GetObjectItemType(cJSON * j_p, char * item_string, int expected_type);
5657

5758
void iperf_dump_fdset(FILE *fp, const char *str, int nfds, fd_set *fds);
5859

0 commit comments

Comments
 (0)