Skip to content

Fix int overflow in serialization size counting - #235

Open
zhouyunding wants to merge 1 commit into
kgabis:masterfrom
zhouyunding:fix/234-serialize-size-overflow
Open

zhouyunding wants to merge 1 commit into
kgabis:masterfrom
zhouyunding:fix/234-serialize-size-overflow

Conversation

@zhouyunding

Copy link
Copy Markdown

Fixes #234

Summary

json_serialization_size() / json_serialization_size_pretty() and the matching serialize functions counted bytes in a signed int internally. For documents whose serialized form exceeds 2^31 bytes the accumulator wraps (signed overflow is UB in C), producing a size of 0 or a small wrapped value. Callers trusting the reported size then either silently lose data (size 0 → serialize returns NULL) or allocate a too-small buffer and crash with a heap buffer overflow while serializing (observed SIGSEGV).

Root cause

Static json_serialize_to_buffer_r() accumulated into int written_total and returned int. Past 2^31 the value wraps to negative / small positive, and the public wrappers translate that into 0/NULL or a bogus small size.

Fix

  • Count internally in size_t and add an explicit pre-addition overflow guard (written_total >= (size_t)-1 - written), failing through the existing (size_t)-1 sentinel → the public wrappers keep returning 0 / NULL / JSONFailure on failure, exactly as documented.
  • Single-point fix: pretty and compact share the same counter, so both paths are covered; all accumulation sites updated.
  • No public API/ABI change: parson.h is untouched.

Behavior change

  • Inputs serializing to > 2^31 bytes previously yielded 0 or a wrapped small size; they now yield the true size (measured 2,165,047,295 for the Integer overflow in json_serialize_to_buffer_r can lead to heap buffer overflow #234 repro, 2,148,532,227 for a synthetic 128-chain case).
  • Genuine overflow (size not representable) now fails cleanly via the existing failure surface instead of wrapping.
  • json_serialize_to_file() still materializes the whole string in memory (peak ≈ 2× payload); this PR does not add streaming output.

Reproducer (before → after, gcc 16.2.0, Windows/UCRT64)

  • Exact Integer overflow in json_serialize_to_buffer_r can lead to heap buffer overflow #234 input (528,383 B, nesting depth 2048): before size = 0 → serialize returns NULL; after size = 2,165,047,295 and the written buffer satisfies strlen+1 == size.
  • Synthetic 256-chain, depth 2048 (true pretty size 4,297,064,450): before reports 2,097,155 (= value mod 2^32) and segfaults (exit 139) when the caller allocates per the reported size; after reports 4,297,064,451 and serializes successfully.

Testing

  • New regression test test_serialization_size_overflow in tests.c (fails on unpatched code, passes after).
  • Full suite via make all (gcc -std=c89 -pedantic-errors, g++, and -DPARSON_FORCE_HASH_COLLISIONS builds): 365/365 passing, zero warnings.
  • Output parity: all tests/*.txt fixtures plus boundary cases are byte-identical before/after for inputs < 2^31.

Note: CI on this PR will show "action required" until a maintainer approves the workflow run (first-time contributor); all results above are from local builds using the same compile flags as CI.


AI assistance disclosure: this patch was developed with AI-assisted tooling (implementation, tests, and independent review rounds); I have reviewed and understand every changed line and take full responsibility for the change.

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.

Integer overflow in json_serialize_to_buffer_r can lead to heap buffer overflow

1 participant