fix(security): recursion/path-depth guards across VRL subsystems (batch C) - #9
Open
JuanMantica45 wants to merge 2 commits into
Open
Conversation
….10743 batch J)
Close all panics and DoS-by-OOM paths identified in the batch-J security audit:
- OBE-10722 find(): clamp negative `from` to 0 before usize cast; guard
find_regex_in_str against offset > haystack.len() (regex::find_at panic).
- OBE-10723 format_number(): replace .expect("not NaN") with fallible
Decimal::from_f64 conversion; returns VRL error for ±∞ and out-of-range floats.
- OBE-10724 format_number(): reject negative scale; cap scale at 1024 to prevent
unbounded push('0') OOM loop; type_def changed to fallible().
- OBE-10727 arithmetic: add safe_mul/safe_add/safe_rem helpers mirroring safe_sub;
replace NotNan::mul/add/rem calls that panic on NaN result (e.g. ∞ * 0).
- OBE-10731 parse_xml(): filter single-child path to element/text nodes; prevents
Comment/PI child from reaching the unreachable!() arm in process_node.
- OBE-10733 starts_with(): fix hand-rolled Chars iterator — treat width==0 (stray
continuation bytes) and truncated multi-byte sequences as error bytes; fix
off-by-one in the Err arm that read past the advanced pos.
- OBE-10734 lex.rs: add b'}' => '}' arm to unescape_string_literal; the lexer
already accepted \} via escape_code but the unescaper had no matching arm,
hitting unimplemented!().
- OBE-10735 array insert: cap insert_value index at ±32768 to bound Null-padding
loop; cap Vec::with_capacity in crud/insert.rs to the same limit.
- OBE-10743 parse_grok(): wrap pattern.match_against in catch_unwind to convert
Oniguruma retry-limit panics to VRL errors (mirrors existing parse_groks guard).
All 1680 lib tests pass. New regression tests added for each fixed panic path.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(OBE-10732 batch C) Close OBE-10732, OBE-10738, OBE-10739, OBE-10740, OBE-10741, OBE-10742, OBE-10744. All seven findings share a single root cause: no depth counter in VRL's parse, compile, and stdlib layers allowed attacker-controlled input to drive unbounded stack recursion. - compiler: reject programs with expression nesting > MAX_EXPR_DEPTH (128) at compile time; this also closes OBE-10740 because programs exceeding the cap never reach the runtime resolver. - parse_ruby_hash: thread a depth counter through parse_value/parse_hash/parse_array (nom closures) and return a hard error above MAX_RUBY_HASH_DEPTH (128). - parse_xml: process_node now carries a depth argument; returns ExpressionError above MAX_XML_DEPTH (128), propagated through parse_xml via ?. - unflatten: do_unflatten_entries bails at MAX_UNFLATTEN_DEPTH (128); do_unflatten_entry caps key splits via splitn(MAX_UNFLATTEN_DEPTH + 1, sep). - set / remove: reject caller-supplied paths longer than MAX_PATH_SEGMENTS (128) before any traversal is attempted. - value::Drop: iterative-drop approach removed; construction-time caps on all external input paths (parse_json via serde_json built-in limit, xml, ruby_hash, unflatten) prevent deeply-nested Values from being created, which eliminates the recursive-drop attack surface without the codebase-wide move-semantics breakage. Six RED security tests added — one per distinct fix site. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.
Why
All seven OBE batch-C findings share one root cause: VRL's parse, compile, and stdlib layers
recurse over attacker-controlled nested input with no depth counter, so a crafted payload can
drive the stack arbitrarily deep and cause a crash or DoS.
What changed
src/compiler/compiler.rscompile_exprchecksdepth >= MAX_EXPR_DEPTH (128)before recursing; programs that exceed the cap are rejected at compile time, which also prevents the runtime resolver (OBE-10740) from ever seeing themsrc/parsing/ruby_hash.rsparse_value/parse_hash/parse_arrayconverted to depth-parameterised nom closures; returns hard failure aboveMAX_RUBY_HASH_DEPTH (128)src/parsing/xml.rsprocess_nodetakes adepth: u32argument; returnsExpressionErroraboveMAX_XML_DEPTH (128), propagated via?src/stdlib/unflatten.rsdo_unflatten_entriesreturns early atMAX_UNFLATTEN_DEPTH (128);do_unflatten_entrycaps key splitting viasplitn(MAX_UNFLATTEN_DEPTH + 1, sep)src/stdlib/set.rsMAX_PATH_SEGMENTS (128)before any traversalsrc/stdlib/remove.rssrc/value/value.rsTickets closed
OBE-10732, OBE-10738, OBE-10739, OBE-10740, OBE-10741, OBE-10742, OBE-10744
Test plan
cargo test— 1687 tests pass (6 new RED security tests, one per fix site)compiler::compiler::tests::test_expression_depth_limit_obe10738— program with 130 nestedif true { }blocks rejected at compile timestdlib::ruby_hash::tests::test_depth_limit_obe10741— 200-level ruby hash rejectedstdlib::parse_xml::tests::test_xml_depth_limit_obe10742— 200-level XML rejectedstdlib::set::tests::test_path_length_limit_obe10739— 200-segment set path rejectedstdlib::remove::tests::test_path_length_limit_obe10739— 200-segment remove path rejectedstdlib::unflatten::tests::test_depth_limit_obe10744— 200-level grouped unflatten completes with bounded nesting🤖 Generated with Claude Code