Skip to content

Bound the results-exchange JSON_read to reject an oversized length - #2073

Open
pau-hedgehog wants to merge 1 commit into
esnet:masterfrom
pau-hedgehog:pau/bound-results-json-read
Open

pau-hedgehog wants to merge 1 commit into
esnet:masterfrom
pau-hedgehog:pau/bound-results-json-read

Conversation

@pau-hedgehog

Copy link
Copy Markdown

PLEASE NOTE the following text from the iperf3 license. Submitting a
pull request to the iperf3 repository constitutes "[making]
Enhancements available...publicly":

You are under no obligation whatsoever to provide any bug fixes, patches, or
upgrades to the features, functionality or performance of the source code
("Enhancements") to anyone; however, if you choose to make your Enhancements
available either publicly, or directly to Lawrence Berkeley National
Laboratory, without imposing a separate written license agreement for such
Enhancements, then you hereby grant the following license: a non-exclusive,
royalty-free perpetual license to install, use, modify, prepare derivative
works, incorporate into other computer software, distribute, and sublicense
such enhancements or derivative works thereof, in binary and source code form.

The complete iperf3 license is available in the LICENSE file in the
top directory of the iperf3 source tree.

get_results() passed 0 as max_size to JSON_read(), disabling its bounds check on the results-exchange read. This adds a new MAX_RESULTS_JSON_STRING bound (64 MiB, generous enough for --get-server-output on a long, many-stream test) and uses it in that call, matching the treatment the params-read call already gets from MAX_PARAMS_JSON_STRING a few hundred lines earlier in the same file.

Tested locally: built and ran make check (all 5 unit tests pass), then ran a real client/server pair with the patched binary covering bidir multi-stream transfer and --get-server-output, confirming get_results() still parses legitimate results correctly under the new bound.

get_results() passed 0 as max_size to JSON_read(), disabling its
bounds check on the final results-exchange read. A misread
SERVER_ERROR control byte could then decode as a ~4 GB length and
turn a server-side abort into a misleading client-side ENOMEM instead
of the real error. Add MAX_RESULTS_JSON_STRING (64 MiB) and use it
here, matching the treatment the params-read call already gets a few
hundred lines earlier in the same file.

Fixes esnet#2072

Signed-off-by: Pau Capdevila <pau@githedgehog.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant