Skip to content

cap recursion depth in yaml and toml generic value readers#2660

Closed
uwezkhan wants to merge 1 commit into
stephenberry:mainfrom
uwezkhan:yaml-toml-recursion-depth
Closed

cap recursion depth in yaml and toml generic value readers#2660
uwezkhan wants to merge 1 commit into
stephenberry:mainfrom
uwezkhan:yaml-toml-recursion-depth

Conversation

@uwezkhan

Copy link
Copy Markdown
Contributor

The YAML and TOML readers parse a generic value by dispatching on the first byte and recursing back through the variant reader for each nested element. Block-style nesting is bounded by the indent stack, but flow collections ([ ], { }) and TOML arrays never touch that stack, so they recurse one level per opening bracket with nothing counting depth. A document that is a few thousand [ therefore overflows the call stack and crashes (SIGSEGV) when read into glz::generic, which is reachable from any untrusted YAML or TOML input.

Before, flow nesting could only stop at the stack limit. After, the variant reader (the single point every nested generic value passes through) takes the same depth_guard the BSON and JSONB readers already use, so a value nested past max_recursive_depth_limit returns exceeded_max_recursive_depth rather than recursing further. Guarding the variant reader instead of each container parser keeps it to one site per format and also covers the block mappings reached from flow context, which the indent stack alone misses. The cost is a fixed 256 level cap on generic values, the same limit the block parsers and the binary readers already enforce.

@stephenberry

Copy link
Copy Markdown
Owner

Thanks @uwezkhan — your commit is carried into #2662, which builds on it with depth guards for the TOML dotted-key/table-header and YAML skip paths and unifies depth_guard onto a single shared copy. Closing in favor of #2662; your authorship is preserved in the commit history.

@uwezkhan

Copy link
Copy Markdown
Contributor Author

Sounds good, thanks for folding it in. #2662 covers more of the recursive paths than my version did, the dotted-key/table-header descent and skip_flow_content were both reachable and I'd only guarded the variant choke point. Collapsing onto one shared depth_guard is cleaner too. Happy with this landing in #2662.

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