Skip to content

Avoid overflow when growing parser cells - #55

Closed
Marti2203 wants to merge 1 commit into
cktan:mainfrom
Marti2203:fix/cell-growth-capacity-overflow
Closed

Marti2203 wants to merge 1 commit into
cktan:mainfrom
Marti2203:fix/cell-growth-capacity-overflow

Conversation

@Marti2203

Copy link
Copy Markdown

Summary

  • compute the 30% cell growth margin in int64_t
  • return allocation failure when the resulting capacity does not fit in int
  • test the last representable and first unrepresentable capacities without allocating gigabytes

toml_parse_file() grows its input buffer through cell_realloc(). At a requested size of 1,651,910,422 bytes, size * 1.3 + 100 exceeds INT_MAX; converting that floating-point value to int has undefined behavior.

Validation

  • make -C test/pool clean test
  • make -C test/source test
  • make -C test/equiv test
  • clang-format --dry-run --Werror src/tomlc17.c test/pool/test1.c
  • git diff --check

The remaining project suites also passed through merge; the aggregate make test then stopped in stdtest because the external go/toml-test commands are not installed in this environment.

cell_realloc() computes a 30% growth margin and stores the result in an int.
For sizes of 1,651,910,422 bytes or greater, the floating-to-integer
conversion is outside the representable range and has undefined behavior.
toml_parse_file() can reach this while growing its input buffer.

Compute the capacity exactly in int64_t and report allocation failure when it
cannot be represented. Cover both sides of the boundary without allocating a
multi-gigabyte buffer.
@cktan

cktan commented Oct 4, 2026

Copy link
Copy Markdown
Owner

Thank you for this, @Marti2203 — the analysis was spot on. size * 1.3 + 100 does exceed INT_MAX from a requested size of 1,651,910,422 bytes, the double-to-int conversion is undefined behavior, and your boundary values and test were both correct (the full suite, including stdtest, passed on your branch here).

We ended up fixing it by a simpler route, so I'm closing this PR as fixed rather than merging it:

  • toml_parse_file() / toml_parse_file_named() / toml_parse_file_ex() now reject files of 1GB or larger with a file is too big error (previously the limit was INT_MAX - CHUNKSZ).
  • With that cap, the largest request cell_realloc() can see from the file buffer is 1GB + 8K, which grows to about 1.4GB — comfortably inside int. The file buffer was the only cell that could get anywhere near the overflow point: tables and arrays are capped at 16k entries (under 1MB of cell storage), and the merge source memo is tiny.

Why this route:

  • cell_realloc() stays untouched, with no new failure branch in a hot allocation path.
  • With the check inside cell_realloc(), a file between ~1.65GB and 2GB would have failed with a misleading out of memory; now oversized input gets one clear, documented error at a round limit.
  • A multi-gigabyte TOML file isn't a use case worth supporting, so a hard 1GB limit costs nothing in practice.

The limit is documented in tomlc17.h and API.md, and the commit credits your report. The fix is committed and will land on main shortly. Thanks again for the careful report and the patch.

@cktan cktan closed this Oct 4, 2026
cktan added a commit that referenced this pull request Oct 4, 2026
cell_realloc() grows a cell to size * 1.3 + 100. For a requested size
of 1,651,910,422 bytes or more that exceeds INT_MAX, and converting it
to int is undefined behavior. The file buffer in toml_parse_file_named()
was the only cell that could get that big: tables and arrays are capped
at 16k entries.

Reject files of 1GB or larger with "file is too big" instead of the old
INT_MAX - CHUNKSZ limit, which keeps the growth margin well within
INT_MAX. Document the limit in the header and API.md.

Reported by Martin Mirchev in #55.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

2 participants