From 0128d0357b7e8916fe39e980e455729bc0e5fd4e Mon Sep 17 00:00:00 2001 From: David Bar-On Date: Sun, 2 Jun 2024 12:41:10 +0300 Subject: [PATCH 1/4] Veify that Params JSON size was received and is resonable --- src/iperf.h | 2 ++ src/iperf_api.c | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/iperf.h b/src/iperf.h index f297587d1..7d14a3453 100644 --- a/src/iperf.h +++ b/src/iperf.h @@ -436,6 +436,8 @@ struct iperf_test #define UDP_BUFFER_EXTRA 1024 +#define MAX_PARAMS_JSON_STRING 8 * 1024 + /* constants for command line arg sanity checks */ #define MB (1024 * 1024) #define MAX_TCP_BUFFER (512 * MB) diff --git a/src/iperf_api.c b/src/iperf_api.c index 565e0b0aa..8a6635222 100644 --- a/src/iperf_api.c +++ b/src/iperf_api.c @@ -2799,8 +2799,9 @@ JSON_read(int fd) * Then read the JSON into a buffer and parse it. Return a parsed JSON * structure, NULL if there was an error. */ - if (Nread(fd, (char*) &nsize, sizeof(nsize), Ptcp) >= 0) { - hsize = ntohl(nsize); + rc = Nread(fd, (char*) &nsize, sizeof(nsize), Ptcp); + hsize = ntohl(nsize); + if (rc == sizeof(nsize) && hsize <= MAX_PARAMS_JSON_STRING) { /* Allocate a buffer to hold the JSON */ strsize = hsize + 1; /* +1 for trailing NULL */ if (strsize) { From 5b0ed630e191dd1ca5a13dcb1bbea1b32b7f1419 Mon Sep 17 00:00:00 2001 From: David Bar-On Date: Sun, 2 Jun 2024 12:48:08 +0300 Subject: [PATCH 2/4] Add check that size is positive --- 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 8a6635222..2d8d4ed42 100644 --- a/src/iperf_api.c +++ b/src/iperf_api.c @@ -2801,7 +2801,7 @@ JSON_read(int fd) */ rc = Nread(fd, (char*) &nsize, sizeof(nsize), Ptcp); hsize = ntohl(nsize); - if (rc == sizeof(nsize) && hsize <= MAX_PARAMS_JSON_STRING) { + if (rc == sizeof(nsize) && hsize > 0 && hsize <= MAX_PARAMS_JSON_STRING) { /* Allocate a buffer to hold the JSON */ strsize = hsize + 1; /* +1 for trailing NULL */ if (strsize) { From d2a6ba63df4c4dbe218ebb383d81e35f0b43a110 Mon Sep 17 00:00:00 2001 From: DavidBar-On Date: Thu, 12 Sep 2024 14:45:28 +0300 Subject: [PATCH 3/4] Changes per reviewer comments (with rebase) --- src/iperf_api.c | 55 +++++++++++++++++++++++++++---------------------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/src/iperf_api.c b/src/iperf_api.c index 2d8d4ed42..30fa13555 100644 --- a/src/iperf_api.c +++ b/src/iperf_api.c @@ -2800,35 +2800,40 @@ JSON_read(int fd) * structure, NULL if there was an error. */ rc = Nread(fd, (char*) &nsize, sizeof(nsize), Ptcp); - hsize = ntohl(nsize); - if (rc == sizeof(nsize) && hsize > 0 && hsize <= MAX_PARAMS_JSON_STRING) { - /* Allocate a buffer to hold the JSON */ - strsize = hsize + 1; /* +1 for trailing NULL */ - if (strsize) { - str = (char *) calloc(sizeof(char), strsize); - if (str != NULL) { - rc = Nread(fd, str, hsize, Ptcp); - if (rc >= 0) { - /* - * We should be reading in the number of bytes corresponding to the - * length in that 4-byte integer. If we don't the socket might have - * prematurely closed. Only do the JSON parsing if we got the - * correct number of bytes. - */ - if (rc == hsize) { - json = cJSON_Parse(str); - } - else { - printf("WARNING: Size of data read does not correspond to offered length\n"); - } - } - } - free(str); + if (rc == sizeof(nsize)) { + hsize = ntohl(nsize); + if (hsize > 0 && hsize <= MAX_PARAMS_JSON_STRING) { + /* Allocate a buffer to hold the JSON */ + strsize = hsize + 1; /* +1 for trailing NULL */ + if (strsize) { + str = (char *) calloc(sizeof(char), strsize); + if (str != NULL) { + rc = Nread(fd, str, hsize, Ptcp); + if (rc >= 0) { + /* + * We should be reading in the number of bytes corresponding to the + * length in that 4-byte integer. If we don't the socket might have + * prematurely closed. Only do the JSON parsing if we got the + * correct number of bytes. + */ + if (rc == hsize) { + json = cJSON_Parse(str); + } + else { + printf("WARNING: JSON size of data read does not correspond to offered length\n"); + } + } + free(str); + } + } } else { - printf("WARNING: Data length overflow\n"); + printf("WARNING: JSON data length overflow\n"); } } + else { + printf("WARNING: Failed to read JSN data size\n"); + } return json; } From dab301f163a078b5a6931d973972efdfcc857659 Mon Sep 17 00:00:00 2001 From: DavidBar-On Date: Sun, 15 Sep 2024 11:24:18 +0300 Subject: [PATCH 4/4] Call warning() instead of printf("WARINING: ....") --- src/iperf_api.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/iperf_api.c b/src/iperf_api.c index 30fa13555..096cfe27f 100644 --- a/src/iperf_api.c +++ b/src/iperf_api.c @@ -2820,7 +2820,7 @@ JSON_read(int fd) json = cJSON_Parse(str); } else { - printf("WARNING: JSON size of data read does not correspond to offered length\n"); + warning("JSON size of data read does not correspond to offered length"); } } free(str); @@ -2828,11 +2828,11 @@ JSON_read(int fd) } } else { - printf("WARNING: JSON data length overflow\n"); + warning("JSON data length overflow"); } } else { - printf("WARNING: Failed to read JSN data size\n"); + warning("Failed to read JSON data size"); } return json; }