Harden container depth

[NovemberZulu]

Issue #, if available: N/A

Description of changes:

Summary

max_container_depth has been present in ION_READER_OPTIONS and ION_WRITER_OPTIONS since the initial commit, but no code ever compared it against a live container depth. It was defaulted, partially validated, and documented as enforced, yet had no effect. As a result deeply nested input was accepted to arbitrary depth, and the library's recursive helpers could exhaust the C stack.

ion process segfaulted on a 30 KB file consisting only of [ characters. The brackets did not even need to be balanced.

These commits enforce the option, raise its default from 10 to 1000, bound the recursive helpers independently, and fix a related bug that capped the text writer at 80 containers with a misleading IERR_NO_MEMORY.

Motivation

Three separate defects, all reachable from untrusted input:

1. The option was inert. With default options (max_container_depth == 10), a plain ion_reader_step_in loop descends 100,000 levels and reports no error. All 11 references to the field were writes or comments; there were zero reads for comparison.

2. Unbounded recursion. _ion_writer_write_all_values_helper is mutually recursive with _ion_writer_write_one_value_helper, and _ion_event_stream_read_all_recursive with ion_event_stream_read, consuming a C stack frame per nesting level with no guard. On an 8 MB stack, ion process crashed between 25,000 and 30,000 levels.

3. The text writer failed at 80 containers. Its container stack is allocated from the writer's temp buffer, a bump allocator with no free. _ion_writer_text_grow_stack doubles the stack and abandons the previous array, so nesting failed with IERR_NO_MEMORY once the fixed 1,584-byte buffer was exhausted, entirely unrelated to max_container_depth.

Defect 3 also coupled nesting depth to an unrelated option, since the buffer was sized from max_annotation_count:

max_annotation_count	containers before failure
10 (default)	80
100	320
1000	2560

The binary writer, which uses heap-backed collections, had no such limit, so the effective depth silently differed by output format. The reported IERR_NO_MEMORY was misleading; the process was not out of memory.

Changes

Five commits, each independently reviewable:

7a6235f Add container depth constants and IERR_STACK_OVERFLOW

Adds DEFAULT_MAX_CONTAINER_DEPTH (1000) and ION_MAX_RECURSION_DEPTH (5000), plus a new error code. No behavior change on its own.

DEFAULT_MAX_CONTAINER_DEPTH is deliberately a new constant rather than a change to DEFAULT_WRITER_STACK_DEPTH. That existing constant also defines ION_EXTRACTOR_MAX_PATH_LENGTH_DEFAULT and therefore sizes fixed-length arrays: raising it to 1000 would grow ION_EXTRACTOR._path_components from 3,840 to 384,000 bytes and place a 24,000-byte array on the stack in ion_extractor.c.

aae98c7 Enforce max_container_depth in reader and writer

Enforces the option at the two points that already track depth, _ion_reader_step_in_helper and _ion_writer_start_container_helper, plus the symbol-table intercept path which increments depth separately. Exceeding it now fails with IERR_STACK_OVERFLOW.

Raises the default from 10 to 1000. A limit of 10 rejects good/equivs/lists.ion from ion-tests, which nests 23 deep; that vector passes today only because the test harness raises the limit to 100. 1000 is 10x that harness value.

Also adds _ion_writer_validate_options, mirroring the reader, so an out-of-range max_container_depth or max_annotation_count is rejected at open rather than silently failing every container. The writer previously performed no options validation at all.

Corrects the ion_extractor.h comment that documented this limit as already enforced.

40babc3 Size the writer temp buffer for the configured container depth

Reserves space for the configured depth plus the doubling waste, and caps growth at max_container_depth so the failure is IERR_STACK_OVERFLOW. A 64-bit intermediate rejects a max_container_depth whose buffer would overflow SIZE. Also honors options.temp_buffer_size, which was defaulted but never read.

Text and binary writers now both stop at exactly max_container_depth.

92ea701 Bound the recursive reader/writer consumers independently

max_container_depth cannot bound these helpers, because a caller may legitimately configure a depth far beyond what the stack can hold, which silently re-arms the crash. With max_container_depth at 200,000, ion_writer_write_all_values still segfaulted on 150,000-deep input. Both are now guarded against fixed ceilings using the depth each already tracks.

The extractor's _ion_extractor_match_helper needs no such guard: its recursion is bounded by max_path_length, which ion_extractor_open validates against the compile-time ION_EXTRACTOR_MAX_PATH_LENGTH (10 by default). Verified with 200,000-deep binary input at a reader limit of 1,000,000.

53fb551 Add container depth tests; fix multi-character literals in existing test

DeeplyNestedContainersDoNotCrash built its input with '[ ' and ' ]', which are multi-character character constants rather than strings. The first appended a single space and the second a lone ], so the intended outer list was absent and the document ended with an unmatched bracket. The test passed for the wrong reason. It also relied on step_in succeeding 1000 levels deep, which only worked because the limit was unenforced, so it now sets an explicit limit and still exercises the scanner's skip-depth guard.

Adds six tests covering the reader and writer limits, the writer's independence from max_annotation_count, recursion in ion_writer_write_all_values, and options validation.

Verification

• Full suite passes: 3003 tests, 47 suites, under both Release and ReleaseSanitizers (ASan + UBSan clean).
• All 2626 ion-tests vector cases pass, including the 982 BadVector cases that must still be rejected.
• ion process on 30 KB of [ characters and on a 100,000-deep balanced document now exit with IERR_STACK_OVERFLOW and a diagnostic instead of SIGSEGV.
• good/equivs/lists.ion (23 deep) parses at default options for the first time.
• Ordinary data round-trips unchanged ({a:1,b:[2,3],c:{d:"x"}} → binary → text).
• The new tests were run against a build containing only the constants commit and none of the fixes: three fail on assertions and WriteAllValuesDoesNotExhaustTheStack segfaults, confirming each targets a real defect rather than passing vacuously.

Compatibility

This changes observable behavior. Callers who relied on the limit being unenforced will now receive IERR_STACK_OVERFLOW beyond 1000 levels of nesting, where previously nesting was unbounded. A default of 10 was not a viable choice, since it rejects Ion's own conformance data.

Because the temp buffer is now sized from max_container_depth, per-writer allocation grows with that option: roughly 50 KB at the new default, up from 1,584 bytes. A caller setting an extreme value gets a proportionally large allocation, rejected only if it would overflow SIZE. An explicit upper bound on the option may be worth considering as a follow-up.

Follow-ups not included here

• The cleaner fix for the temp buffer is to stop leaking on _ion_writer_text_grow_stack altogether, by allocating the container stack from the writer's owner pool rather than the bump arena. That is a structural change and is deliberately out of scope.
• UBSan reports signed-integer overflow from left shifts at ion_symbol_table.c:2184, in a hash computation. It is unrelated to container depth and does not fail the suite, but it is in the area touched by the two preceding hardening commits and may warrant a separate look.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[tgregg]

The ion-test-driver failures are because 5 lines were added to ion_event_stream.cpp, causing the error messages compared against the previous commit to contain line numbers increased by 5 but otherwise identical. This is not a merge blocker. Subsequent PRs will compare against this revision (using the new line numbers).