Fix integer overflow in vendored json-parser allocation size - #1296
Fix integer overflow in vendored json-parser allocation size#1296Ion (Socialpranker) wants to merge 2 commits into
Conversation
new_value() in clients/cloud/c/json.c multiplies value->u.array.length and value->u.object.length by sizeof(json_value*) / sizeof(json_object_entry) before passing the result to json_alloc(), with no check that the multiplication itself fits in the allocator's size type. On a platform where this overflows, the allocation comes back undersized while the parser still writes array.length / object.length elements into it, producing a heap buffer overflow from attacker-controlled JSON input. json_alloc() already guards against overflowing the running memory total via state->ulong_max, but that only helps once the multiplied size is passed in - it doesn't stop the multiplication from wrapping first. This adds an explicit bound check before each multiplication, using the same state->ulong_max the file already computes for this purpose, matching the existing style. Verified: reviewed clients/cloud/c/json.c against upstream json-parser/json-parser, where this exact class of overflow was fixed via a similar bounds check. Compiled clients/cloud/c/json.c standalone with gcc -Wall -Wextra; no new warnings versus the unpatched file.
The previous guard compared length against ulong_max, but values_size that stores the multiplication result is declared as a plain int, so a length between INT_MAX/sizeof(entry) and ulong_max/sizeof(entry) still overflows values_size itself before the ulong_max check would ever catch it. Guard against INT_MAX/sizeof(entry) instead, matching the type that actually holds the product.
|
Pushed a follow-up commit: the object-branch guard checked against |
|
Closing this as stale on my side — no reflection on the change itself. The default branch is currently driven entirely by automation, and the last human-merged pull request was in August 2025. I'd rather not leave a PR sitting in a queue that isn't being processed. The fix itself remains valid and the branch stays up. If maintenance picks up and this is still useful, feel free to reopen — no need to ask me first. |
1 participant
This patch was written by Claude Sonnet 5 (Anthropic); I reviewed the change and the reasoning before submitting.
clients/cloud/c/json.c vendors the json-parser library. In new_value(), the array and object branches multiply value->u.array.length and value->u.object.length by sizeof(json_value*) and sizeof(json_object_entry) respectively before passing the result to json_alloc(). Neither multiplication is checked for overflow first. If it wraps, json_alloc() receives a small size, the allocation succeeds, and the parser then writes length elements into a buffer that is too small for them, a heap buffer overflow driven entirely by the JSON input being parsed.
json_alloc() does guard state->used_memory against wrapping via state->ulong_max, but that check only runs after the multiplication has already happened, so it cannot catch an overflow in the multiplication itself.
The fix adds a bound check in both branches before the multiplication, using the same state->ulong_max field json_alloc() already computes for the same purpose, so the guard follows the existing convention in the file rather than introducing a new one. On failure it returns 0, which is exactly how allocation failure is already signaled everywhere else in new_value(). The diff is intentionally limited to these two checks, no other code in the file was touched.
I compiled clients/cloud/c/json.c standalone with gcc -Wall -Wextra -std=c99 before and after the change; the warning output is identical (two pre-existing unused-parameter warnings in default_alloc/default_free, unrelated to this fix). I did not build consumer.c/producer.c through the Makefile because that requires librdkafka, which isn't installed in the environment I used, but json.c has no dependency on librdkafka and compiles cleanly on its own.