esnet / esnet/iperf

get_results() passes 0 as max_size to JSON_read(), so a misread control-channel byte can trigger a multi-GB allocation instead of a clean error

Open
#2,072 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
8.8k
Forks
1.5k
PR merge metrics
No merged PRs in 30d

Description

get_results() passes 0 as max_size to JSON_read(), so a misread control-channel byte can trigger a multi-GB allocation instead of a clean error

In src/iperf_api.c, get_results() reads the peer's final results message off the control connection with no size cap.

```
j = JSON_read(test->ctrl_sck, 0);
if (j == NULL) {
i_errno = IERECVRESULTS;
r = -1;
```

JSON_read() treats max_size == 0 as no limit, so whatever 32-bit value arrives as the length prefix goes straight to calloc without a bounds check.

```
hsize = ntohl(nsize);
if (hsize > 0 && (max_size == 0 || hsize <= max_size)) {
/* Allocate a buffer to hold the JSON */
strsize = hsize + 1; /* +1 for trailing NULL */
if (strsize) {
str = (char *) calloc(sizeof(char), strsize);
```

We hit this in practice when a server aborted mid results-exchange. SERVER_ERROR is written to the control socket as a single signed byte, 0xFE, followed by two four-byte error codes. A client already blocked inside get_results() at that moment reads that 0xFE byte plus the top three bytes of the following network-order integer, which are zero for the small values that enum holds, as a big-endian length: exactly 0xFE000000, 3.97 GiB. On a small VM that allocation is refused, calloc fails, JSON_read() returns NULL, and the client reports Cannot allocate memory instead of ever reaching the SERVER_ERROR handler in iperf_client_api.c that would have printed the real reason. So this isn't just a missing bound, it actively destroys the server's own error message on abort.

```
unable to receive results: Cannot allocate memory
```

The params-read call a few hundred lines earlier in the same file already passes a real bound, MAX_PARAMS_JSON_STRING.

```
j = JSON_read(test->ctrl_sck, MAX_PARAMS_JSON_STRING);
```

get_results() should get the same treatment. Opening a PR with a fix.

Longer writeup of how we found this, including the byte-level decoding: https://github.com/githedgehog/toolbox/issues/43

Contributor guide

Open the contributing guide

Research direction

Start in src/iperf_api.c at get_results() and compare its JSON_read() call with the params-read call a few hundred lines earlier, which uses MAX_PARAMS_JSON_STRING. Verify the control-channel length is bounded before allocation and check that an aborted results exchange reaches the existing SERVER_ERROR handling instead of reporting an allocation failure.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
networking, security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.