Fix/json unchecked recursion depth - #220
Merged
fredbi merged 3 commits intoJul 18, 2026
Merged
Conversation
The ordered-JSON adapters (stdlib and easyjson) recursed once per JSON
nesting level with no upper bound, on both the unmarshal and marshal
paths. A sufficiently deep JSON document, or an equally deep in-memory
JSONMapSlice passed to WriteJSON, drove the goroutine stack past its
limit and triggered a non-recoverable `fatal error: stack overflow`
that terminates the process.
The stdlib adapter's lexer is built on encoding/json's streaming
Decoder.Token(), which does not carry the max-nesting-depth guard that
encoding/json.Unmarshal enforces via its scanner (const maxNestingDepth
= 10000). The easyjson adapter had no equivalent guard either.
This change enforces a maximum container nesting depth:
- Unmarshal: the stdlib lexer tracks depth centrally in Delim() (one
increment per '{'/'[', one decrement per '}'/']'); the easyjson path
threads a decreasing budget through unmarshalEasyJSON/asInterface.
Exceeding the limit sets a parse error instead of recursing further.
- Marshal: Adapter.OrderedMarshal and the MapSlice marshal path thread
the depth/budget and return an error past the limit; nested ordered
maps recurse internally so the guard is not lost across a
stdjson.Marshal boundary.
The default limit is 10000, matching encoding/json's decoder. It is
configurable per adapter via the new WithMaxNestingDepth option, wired
through Register's constructor so it takes effect on the pooled
ReadJSON/WriteJSON path (this also makes the easyjson adapter's other
registered options effective, which the pooled constructor previously
dropped).
Deeply nested input now returns an error rather than crashing. Tests
cover object and array nesting, both adapters, the marshal and
unmarshal paths, the configurable limit, and end-to-end ReadJSON/
WriteJSON behavior.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Frédéric BIDON <fredbi@yahoo.com>
The YAML<->JSON conversion walkers recursed once per nesting level with no upper bound. A deeply nested in-memory structure could therefore drive the goroutine stack past its limit and trigger a non-recoverable `fatal error: stack overflow` that terminates the process: - Encode: YAMLMapSlice.MarshalYAML -> json2yaml/orderedYAML recurse for every nested map/slice/ordered value. - Decode: YAMLToJSON -> transformData recurses over map[any]any/[]any, and yamlNode/yamlSequence/yamlMapping/UnmarshalYAML walk the node tree. The parse entry point (BytesToYAMLDoc via yaml.Unmarshal) was already protected: go.yaml.in/yaml/v3 caps parsing at a max depth of 10000. But YAMLToJSON accepts an arbitrary value (a caller may pass a deep map/slice directly), and the marshal path is not parser-bounded at all, so both could still crash. This change threads a depth counter through all three recursion sets and returns an error once it exceeds the limit. The limit is a fixed 10000, matching go.yaml.in/yaml/v3's own parser and encoding/json's decoder; it is not configurable, consistent with the underlying library, and because MarshalYAML/UnmarshalYAML implement fixed yaml interfaces that cannot carry per-call options. Deeply nested input now returns an error rather than crashing. Tests cover the encode path, the decode path (raw map/slice and parsed document), and a moderate round-trip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Frédéric BIDON <fredbi@yahoo.com>
YAMLToJSON walks the low-level yaml.Node tree itself (to preserve key
order) and resolves anchor/alias references in yamlNode's AliasNode case
by re-walking and re-expanding the aliased subtree on every reference,
with no bound. A small document with a chain of anchors that each
reference the previous one several times therefore expands
exponentially ("alias bomb" / billion laughs): a sub-kilobyte input can
expand to billions of nodes, exhausting memory.
go.yaml.in/yaml/v3 guards against this, but its guard is coupled to the
library's own tree walk and only runs when decoding into Go values.
Decoding into a *yaml.Node short-circuits that walk (decode.go: the
nodeType fast path returns before recursing), so the guard never
engages and the aliases are handed back unexpanded. By doing the
expansion ourselves we inherited the library's job without its bound.
This reproduces the library's excessive-aliasing guard on our walk: a
per-conversion yamlWalker tracks decodeCount and alias-driven aliasCount
and fails once alias expansion is disproportionate to document size,
using the same constants and ratio schedule (allowedAliasRatio) as the
library's decoder.
It also adds explicit alias cycle detection (a set of anchors currently
being expanded), so a self-referential anchor now errors as
"anchor contains itself" instead of being caught only incidentally by
the nesting-depth guard (which produced a ~10000-deep wrapped error).
Tests cover the alias bomb (rejected, not expanded), a self-referential
cycle (clean error), and legitimate bounded alias use (still resolves).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Frédéric BIDON <fredbi@yahoo.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #220 +/- ##
==========================================
- Coverage 92.55% 92.20% -0.36%
==========================================
Files 56 57 +1
Lines 2620 2746 +126
==========================================
+ Hits 2425 2532 +107
- Misses 152 170 +18
- Partials 43 44 +1 ☔ View full report in Codecov by Harness. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change type
Please select: 🆕 New feature or enhancement|🔧 Bug fix'|📃 Documentation update
Short description
Fixes
Full description
Checklist