diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index e14e6294d1..2692ce41d7 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -56,6 +56,9 @@ pub struct Compiler<'a> { // the error from the LHS) fallible_expression_error: Option, + /// Current expression nesting depth, incremented on each compile_expr entry. + depth: u32, + config: CompileConfig, } @@ -102,6 +105,7 @@ impl<'a> Compiler<'a> { external_assignments: vec![], skip_missing_query_target: vec![], fallible_expression_error: None, + depth: 0, config, }; let expressions = compiler.compile_root_exprs(ast, &mut state); @@ -148,7 +152,27 @@ impl<'a> Compiler<'a> { Some(exprs) } + const MAX_EXPR_DEPTH: u32 = 128; + fn compile_expr(&mut self, node: Node, state: &mut TypeState) -> Option { + if self.depth >= Self::MAX_EXPR_DEPTH { + self.diagnostics.push(Box::new(ExpressionError::Error { + message: format!( + "expression nesting depth limit ({}) exceeded", + Self::MAX_EXPR_DEPTH + ), + labels: vec![], + notes: vec![], + })); + return None; + } + self.depth += 1; + let result = self.compile_expr_inner(node, state); + self.depth -= 1; + result + } + + fn compile_expr_inner(&mut self, node: Node, state: &mut TypeState) -> Option { use ast::Expr::{ Abort, Assignment, Container, FunctionCall, IfStatement, Literal, Op, Query, Return, Unary, Variable, @@ -852,3 +876,33 @@ impl<'a> Compiler<'a> { self.skip_missing_query_target.push(query); } } + +#[cfg(test)] +mod tests { + #[test] + fn test_expression_depth_limit_obe10738() { + // OBE-10738: VRL programs with expression nesting > MAX_EXPR_DEPTH must be rejected at + // compile time. Without the fix, the compiler recurses once per expression level and can + // stack overflow on crafted programs. + // + // We spawn with a larger stack because the VRL parser itself is recursive and overflows the + // default thread stack before the compiler's depth check can fire. 32 MB is enough for the + // parser to survive 130 levels while the compiler rejects at MAX_EXPR_DEPTH (128). + // + // With fix: compiler catches at depth 128 → Err. + // Without fix: compiler recurses 130 times and returns Ok → assertion fails. + let depth = 130usize; + let is_err = std::thread::Builder::new() + .stack_size(32 * 1024 * 1024) + .spawn(move || { + let open: String = "if true { ".repeat(depth); + let close: String = " }".repeat(depth); + let program = format!("{open}1{close}"); + crate::compiler::compile(&program, &crate::stdlib::all()).is_err() + }) + .expect("thread spawn failed") + .join() + .expect("thread panicked"); + assert!(is_err, "program with {depth} nested if-blocks must fail compilation (OBE-10738)"); + } +} diff --git a/src/compiler/value/arithmetic.rs b/src/compiler/value/arithmetic.rs index 80b63b30bc..07985b959a 100644 --- a/src/compiler/value/arithmetic.rs +++ b/src/compiler/value/arithmetic.rs @@ -1,6 +1,5 @@ #![deny(clippy::arithmetic_side_effects)] -use std::ops::{Add, Mul, Rem}; use crate::compiler::{ value::{Kind, VrlValueConvert}, @@ -68,6 +67,33 @@ fn safe_sub(lhv: f64, rhv: f64) -> Option { } } +fn safe_add(lhv: f64, rhv: f64) -> Option { + let result = lhv + rhv; + if result.is_nan() { + None + } else { + Some(Value::from_f64_or_zero(result)) + } +} + +fn safe_mul(lhv: f64, rhv: f64) -> Option { + let result = lhv * rhv; + if result.is_nan() { + None + } else { + Some(Value::from_f64_or_zero(result)) + } +} + +fn safe_rem(lhv: f64, rhv: f64) -> Option { + let result = lhv % rhv; + if result.is_nan() { + None + } else { + Some(Value::from_f64_or_zero(result)) + } +} + impl VrlValueArithmetic for Value { /// Similar to [`std::ops::Mul`], but fallible (e.g. `TryMul`). fn try_mul(self, rhs: Self) -> Result { @@ -90,7 +116,7 @@ impl VrlValueArithmetic for Value { } Value::Float(lhv) => { let rhs = rhs.try_into_f64().map_err(|_| err())?; - lhv.mul(rhs).into() + safe_mul(*lhv, rhs).ok_or_else(err)? } Value::Bytes(lhv) if rhs.is_integer() => { Bytes::from(lhv.repeat(as_usize(rhs.try_integer()?))).into() @@ -134,7 +160,8 @@ impl VrlValueArithmetic for Value { let rhs = rhs .try_into_f64() .map_err(|_| ValueError::Add(Kind::float(), rhs.kind()))?; - lhs.add(rhs).into() + safe_add(*lhs, rhs) + .ok_or(ValueError::Add(Kind::float(), Kind::float()))? } (lhs @ Value::Bytes(_), Value::Null) => lhs, (Value::Bytes(lhs), Value::Bytes(rhs)) => { @@ -230,7 +257,7 @@ impl VrlValueArithmetic for Value { } Value::Float(lhv) => { let rhv = rhs.try_into_f64().map_err(|_| err())?; - lhv.rem(rhv).into() + safe_rem(*lhv, rhv).ok_or_else(err)? } _ => return Err(err()), }; diff --git a/src/parser/lex.rs b/src/parser/lex.rs index 620bbd7cd2..2dbc19fa04 100644 --- a/src/parser/lex.rs +++ b/src/parser/lex.rs @@ -1290,6 +1290,7 @@ fn unescape_string_literal(mut s: &str) -> String { b't' => '\t', b'0' => '\0', b'{' => '{', + b'}' => '}', _ => unimplemented!("invalid escape"), }; diff --git a/src/parsing/ruby_hash.rs b/src/parsing/ruby_hash.rs index 4ed63afac4..712825a9c0 100644 --- a/src/parsing/ruby_hash.rs +++ b/src/parsing/ruby_hash.rs @@ -12,8 +12,10 @@ use nom::{ }; use std::num::ParseIntError; +const MAX_RUBY_HASH_DEPTH: usize = 128; + pub(crate) fn parse_ruby_hash(input: &str) -> ExpressionResult { - let result = parse_hash(input) + let result = parse_hash(0)(input) .map_err(|err| match err { nom::Err::Error(err) | nom::Err::Failure(err) => { // Create a descriptive error message if possible. @@ -139,64 +141,84 @@ fn parse_key<'a, E: HashParseError<&'a str>>(input: &'a str) -> IResult<&'a str, ))(input) } -fn parse_array<'a, E: HashParseError<&'a str>>(input: &'a str) -> IResult<&'a str, Value, E> { - context( - "array", - map( - preceded( - char('['), - cut(terminated( - separated_list0(preceded(sp, char(',')), parse_value), - preceded(sp, char(']')), - )), +fn parse_array<'a, E: HashParseError<&'a str>>( + depth: usize, +) -> impl FnMut(&'a str) -> IResult<&'a str, Value, E> { + move |input| { + context( + "array", + map( + preceded( + char('['), + cut(terminated( + separated_list0(preceded(sp, char(',')), parse_value(depth + 1)), + preceded(sp, char(']')), + )), + ), + Value::Array, ), - Value::Array, - ), - )(input) + )(input) + } } fn parse_key_value<'a, E: HashParseError<&'a str>>( - input: &'a str, -) -> IResult<&'a str, (KeyString, Value), E> { - separated_pair( - preceded(sp, parse_key), - cut(preceded(sp, alt((tag(":"), tag("=>"))))), - parse_value, - )(input) + depth: usize, +) -> impl FnMut(&'a str) -> IResult<&'a str, (KeyString, Value), E> { + move |input| { + separated_pair( + preceded(sp, parse_key), + cut(preceded(sp, alt((tag(":"), tag("=>"))))), + parse_value(depth), + )(input) + } } -fn parse_hash<'a, E: HashParseError<&'a str>>(input: &'a str) -> IResult<&'a str, Value, E> { - context( - "map", - map( - preceded( - char('{'), - cut(terminated( - map( - separated_list0(preceded(sp, char(',')), parse_key_value), - |tuple_vec| tuple_vec.into_iter().collect(), - ), - preceded(sp, char('}')), - )), +fn parse_hash<'a, E: HashParseError<&'a str>>( + depth: usize, +) -> impl FnMut(&'a str) -> IResult<&'a str, Value, E> { + move |input| { + context( + "map", + map( + preceded( + char('{'), + cut(terminated( + map( + separated_list0(preceded(sp, char(',')), parse_key_value(depth + 1)), + |tuple_vec| tuple_vec.into_iter().collect(), + ), + preceded(sp, char('}')), + )), + ), + Value::Object, ), - Value::Object, - ), - )(input) + )(input) + } } -fn parse_value<'a, E: HashParseError<&'a str>>(input: &'a str) -> IResult<&'a str, Value, E> { - preceded( - sp, - alt(( - parse_nil, - parse_hash, - parse_array, - map(parse_colon_key, Value::from), - map(parse_bytes, Value::Bytes), - map(double, |value| Value::Float(NotNan::new(value).unwrap())), - map(parse_boolean, Value::Boolean), - )), - )(input) +fn parse_value<'a, E: HashParseError<&'a str>>( + depth: usize, +) -> impl FnMut(&'a str) -> IResult<&'a str, Value, E> { + move |input| { + if depth > MAX_RUBY_HASH_DEPTH { + return Err(nom::Err::Failure(E::from_error_kind( + input, + nom::error::ErrorKind::TooLarge, + ))); + } + preceded( + sp, + alt(( + parse_nil, + parse_hash(depth), + parse_array(depth), + map(parse_colon_key, Value::from), + map(parse_bytes, Value::Bytes), + map(double, |value| Value::Float(NotNan::new(value).unwrap())), + map(parse_boolean, Value::Boolean), + )), + )(input) + } } #[cfg(test)] @@ -339,4 +361,16 @@ mod tests { fn test_non_hash() { assert!(parse_ruby_hash(r#""hello world""#).is_err()); } + + #[test] + fn test_depth_limit_obe10741() { + // OBE-10741: deeply nested ruby hash from attacker-controlled input must be rejected. + // Without the fix, parsing 200 levels of nesting recurses through parse_value/parse_hash + // and can cause stack overflow or DoS. + let open: String = "{:k => ".repeat(200); + let close: String = "}".repeat(200); + let input = format!("{open}1{close}"); + let result = parse_ruby_hash(&input); + assert!(result.is_err(), "hash with 200 nesting levels must be rejected (OBE-10741)"); + } } diff --git a/src/parsing/xml.rs b/src/parsing/xml.rs index 1bbfd1dc15..cb0381b988 100644 --- a/src/parsing/xml.rs +++ b/src/parsing/xml.rs @@ -91,14 +91,20 @@ pub(crate) fn parse_xml(value: Value, options: ParseOptions) -> Resolved { // Trim whitespace around XML elements, if applicable. let parse = if trim { trim_xml(&string) } else { string }; let doc = Document::parse(&parse).map_err(|e| format!("unable to parse xml: {e}"))?; - let value = process_node(doc.root(), &config); + let value = process_node(doc.root(), &config, 0)?; Ok(value) } +const MAX_XML_DEPTH: u32 = 128; + /// Process an XML node, and return a VRL `Value`. -fn process_node(node: Node, config: &ParseXmlConfig) -> Value { +fn process_node(node: Node, config: &ParseXmlConfig, depth: u32) -> ExpressionResult { + if depth > MAX_XML_DEPTH { + return Err("xml nesting depth limit (128) exceeded".into()); + } + // Helper to recurse over a `Node`s children, and build an object. - let recurse = |node: Node| -> ObjectMap { + let recurse = |node: Node| -> ExpressionResult { let mut map = BTreeMap::new(); // Expand attributes, if required. @@ -119,7 +125,7 @@ fn process_node(node: Node, config: &ParseXmlConfig) -> Value { }; // Transform the node into a VRL `Value`. - let value = process_node(n, config); + let value = process_node(n, config, depth + 1)?; // If the key already exists, add it. Otherwise, insert. match map.entry(name) { @@ -143,11 +149,11 @@ fn process_node(node: Node, config: &ParseXmlConfig) -> Value { } } - map + Ok(map) }; match node.node_type() { - NodeType::Root => Value::Object(recurse(node)), + NodeType::Root => Ok(Value::Object(recurse(node)?)), NodeType::Element => { match ( @@ -155,37 +161,40 @@ fn process_node(node: Node, config: &ParseXmlConfig) -> Value { node.attributes().len().is_zero(), ) { // If the node has attributes, *always* recurse to expand default keys. - (_, false) if config.include_attr => Value::Object(recurse(node)), + (_, false) if config.include_attr => Ok(Value::Object(recurse(node)?)), // If a text key should be used, always recurse. - (true, true) => Value::Object(recurse(node)), + (true, true) => Ok(Value::Object(recurse(node)?)), // Otherwise, check the node count to determine what to do. _ => match node.children().count() { // For a single node, 'flatten' the object if necessary. 1 => { - // Expect a single element. - let node = node.children().next().expect("expected 1 XML node"); - - // If the node is an element, treat it as an object. - if node.is_element() { - let mut map = BTreeMap::new(); - - map.insert( - node.tag_name().name().to_string().into(), - process_node(node, config), - ); - - Value::Object(map) - } else { - // Otherwise, 'flatten' the object by continuing processing. - process_node(node, config) + // Skip non-element/non-text nodes (e.g. comments, PIs) to prevent + // passing them to process_node which cannot handle them. + let child = node + .children() + .find(|n| n.is_element() || n.is_text()); + match child { + Some(node) if node.is_element() => { + let mut map = BTreeMap::new(); + + map.insert( + node.tag_name().name().to_string().into(), + process_node(node, config, depth + 1)?, + ); + + Ok(Value::Object(map)) + } + Some(node) => process_node(node, config, depth + 1), + // Only child is a comment or PI — treat as empty element. + None => Ok(Value::Object(recurse(node)?)), } } // For 2+ nodes, expand. - _ => Value::Object(recurse(node)), + _ => Ok(Value::Object(recurse(node)?)), }, } } - NodeType::Text => process_text(node.text().expect("expected XML text node"), config), + NodeType::Text => Ok(process_text(node.text().expect("expected XML text node"), config)), _ => unreachable!("shouldn't be other XML nodes"), } } diff --git a/src/stdlib/find.rs b/src/stdlib/find.rs index 8e9bc0b3ca..5f690392bd 100644 --- a/src/stdlib/find.rs +++ b/src/stdlib/find.rs @@ -3,7 +3,7 @@ use crate::compiler::prelude::*; #[allow(clippy::cast_possible_wrap)] fn find(value: Value, pattern: Value, from: Option) -> Resolved { let from = match from { - Some(value) => value.try_integer()?, + Some(value) => value.try_integer()?.max(0), None => 0, } as usize; @@ -75,6 +75,9 @@ struct FindFn { impl FindFn { fn find_regex_in_str(value: &str, regex: ValueRegex, offset: usize) -> Option { + if offset > value.len() { + return None; + } regex.find_at(value, offset).map(|found| found.start()) } @@ -178,5 +181,24 @@ mod tests { want: Err("expected string or regex, got integer"), tdef: TypeDef::integer().infallible(), } + + // OBE-10722: negative from wraps to huge usize, panics in regex::find_at + negative_from_string { + args: func_args![value: "foobar", pattern: "bar", from: -1_i64], + want: Ok(value!(3)), + tdef: TypeDef::integer().infallible(), + } + + negative_from_regex { + args: func_args![value: "foobar", pattern: Value::Regex(Regex::new("bar").unwrap().into()), from: -10_i64], + want: Ok(value!(3)), + tdef: TypeDef::integer().infallible(), + } + + from_past_end { + args: func_args![value: "foobar", pattern: Value::Regex(Regex::new("bar").unwrap().into()), from: 100_i64], + want: Ok(value!(-1)), + tdef: TypeDef::integer().infallible(), + } ]; } diff --git a/src/stdlib/format_number.rs b/src/stdlib/format_number.rs index 1776c6f8cd..f949306dd3 100644 --- a/src/stdlib/format_number.rs +++ b/src/stdlib/format_number.rs @@ -9,7 +9,8 @@ fn format_number( ) -> Resolved { let value: Decimal = match value { Value::Integer(v) => v.into(), - Value::Float(v) => Decimal::from_f64(*v).expect("not NaN"), + Value::Float(v) => Decimal::from_f64(*v) + .ok_or("cannot convert float to decimal: value is infinite or out of range")?, value => { return Err(ValueError::Expected { got: value.kind(), @@ -39,8 +40,14 @@ fn format_number( debug_assert!(parts.len() <= 2); // Manipulate fractional part based on configuration. match scale { + Some(i) if i < 0 => { + return Err(format!("scale must be non-negative, got {i}").into()); + } Some(0) => parts.truncate(1), Some(i) => { + if i > 1024 { + return Err(format!("scale must not exceed 1024, got {i}").into()); + } let i = i as usize; if parts.len() == 1 { @@ -173,7 +180,7 @@ impl FunctionExpression for FormatNumberFn { } fn type_def(&self, _: &state::TypeState) -> TypeDef { - TypeDef::bytes().infallible() + TypeDef::bytes().fallible() } } @@ -188,14 +195,14 @@ mod tests { number { args: func_args![value: 1234.567], want: Ok(value!("1234.567")), - tdef: TypeDef::bytes().infallible(), + tdef: TypeDef::bytes().fallible(), } precision { args: func_args![value: 1234.567, scale: 2], want: Ok(value!("1234.56")), - tdef: TypeDef::bytes().infallible(), + tdef: TypeDef::bytes().fallible(), } @@ -204,7 +211,7 @@ mod tests { scale: 2, decimal_separator: ","], want: Ok(value!("1234,56")), - tdef: TypeDef::bytes().infallible(), + tdef: TypeDef::bytes().fallible(), } more_separators { @@ -213,7 +220,7 @@ mod tests { decimal_separator: ",", grouping_separator: " "], want: Ok(value!("1 234,56")), - tdef: TypeDef::bytes().infallible(), + tdef: TypeDef::bytes().fallible(), } big_number { @@ -222,34 +229,66 @@ mod tests { decimal_separator: ",", grouping_separator: "."], want: Ok(value!("11.222.333.444,567")), - tdef: TypeDef::bytes().infallible(), + tdef: TypeDef::bytes().fallible(), } integer { args: func_args![value: 100.0], want: Ok(value!("100")), - tdef: TypeDef::bytes().infallible(), + tdef: TypeDef::bytes().fallible(), } integer_decimals { args: func_args![value: 100.0, scale: 2], want: Ok(value!("100.00")), - tdef: TypeDef::bytes().infallible(), + tdef: TypeDef::bytes().fallible(), } float_no_decimals { args: func_args![value: 123.45, scale: 0], want: Ok(value!("123")), - tdef: TypeDef::bytes().infallible(), + tdef: TypeDef::bytes().fallible(), } integer_no_decimals { args: func_args![value: 12345, scale: 2], want: Ok(value!("12345.00")), - tdef: TypeDef::bytes().infallible(), + tdef: TypeDef::bytes().fallible(), + } + + // OBE-10723: panic on ±∞ (float not representable as Decimal) + float_infinity { + args: func_args![value: f64::INFINITY], + want: Err("cannot convert float to decimal: value is infinite or out of range"), + tdef: TypeDef::bytes().fallible(), + } + + float_neg_infinity { + args: func_args![value: f64::NEG_INFINITY], + want: Err("cannot convert float to decimal: value is infinite or out of range"), + tdef: TypeDef::bytes().fallible(), + } + + // OBE-10724: OOM / panic on negative or huge scale + negative_scale { + args: func_args![value: 1.0, scale: -1_i64], + want: Err("scale must be non-negative, got -1"), + tdef: TypeDef::bytes().fallible(), + } + + excessive_scale { + args: func_args![value: 1.0, scale: 1025_i64], + want: Err("scale must not exceed 1024, got 1025"), + tdef: TypeDef::bytes().fallible(), + } + + max_allowed_scale { + args: func_args![value: 1.5, scale: 3_i64], + want: Ok(value!("1.500")), + tdef: TypeDef::bytes().fallible(), } ]; } diff --git a/src/stdlib/parse_grok.rs b/src/stdlib/parse_grok.rs index 55a75bd541..376915c0f3 100644 --- a/src/stdlib/parse_grok.rs +++ b/src/stdlib/parse_grok.rs @@ -10,8 +10,11 @@ mod non_wasm { fn parse_grok(value: Value, pattern: Arc) -> Resolved { let bytes = value.try_bytes_utf8_lossy()?; - match pattern.match_against(&bytes) { - Some(matches) => { + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + pattern.match_against(&bytes) + })); + match result { + Ok(Some(matches)) => { let mut result = BTreeMap::new(); for (name, value) in &matches { @@ -20,7 +23,8 @@ mod non_wasm { Ok(Value::from(result)) } - None => Err("unable to parse input with grok pattern".into()), + Ok(None) => Err("unable to parse input with grok pattern".into()), + Err(_) => Err("grok pattern match failed: regex engine error".into()), } } diff --git a/src/stdlib/parse_xml.rs b/src/stdlib/parse_xml.rs index 4fac4980ff..6a657d361a 100644 --- a/src/stdlib/parse_xml.rs +++ b/src/stdlib/parse_xml.rs @@ -514,4 +514,20 @@ mod tests { assert!(object2.known().is_empty()); assert!(object2.unknown_kind().is_any()); } + + #[test] + fn test_xml_depth_limit_obe10742() { + // OBE-10742: deeply nested XML from attacker-controlled input must be rejected. + // Without the fix, process_node recurses for every element level and can stack overflow. + let depth = 200usize; + let open: String = (0..depth).map(|i| format!("")).collect(); + let close: String = (0..depth).rev().map(|i| format!("")).collect(); + let xml = format!("{open}hi{close}"); + let value = Value::Bytes(xml.into()); + let options = crate::parsing::xml::ParseOptions::default(); + let result = crate::parsing::xml::parse_xml(value, options); + assert!(result.is_err(), "XML with {depth} nesting levels must be rejected (OBE-10742)"); + let msg = result.unwrap_err().to_string(); + assert!(msg.contains("depth limit"), "error must mention depth limit: {msg}"); + } } diff --git a/src/stdlib/remove.rs b/src/stdlib/remove.rs index 50a0041944..8673c2286d 100644 --- a/src/stdlib/remove.rs +++ b/src/stdlib/remove.rs @@ -1,9 +1,19 @@ use crate::compiler::prelude::*; use crate::path::{OwnedSegment, OwnedValuePath}; +const MAX_PATH_SEGMENTS: usize = 128; + fn remove(path: Value, compact: Value, mut value: Value) -> Resolved { let path = match path { Value::Array(path) => { + if path.len() > MAX_PATH_SEGMENTS { + return Err(format!( + "path has {} segments, max is {MAX_PATH_SEGMENTS}", + path.len() + ) + .into()); + } + let mut lookup = OwnedValuePath::root(); for segment in path { @@ -212,4 +222,15 @@ mod tests { tdef: TypeDef::object(Collection::any()).fallible(), } ]; + + #[test] + fn test_path_length_limit_obe10739() { + // OBE-10739: attacker-controlled path with > MAX_PATH_SEGMENTS segments must be rejected. + // Without the fix this returns Ok and silently accepts arbitrary-depth traversal. + let segments: Vec = (0..200).map(|i| Value::Bytes(format!("k{i}").into())).collect(); + let result = remove(Value::Array(segments), Value::Boolean(false), Value::Object(ObjectMap::new())); + assert!(result.is_err(), "path with 200 segments must be rejected (OBE-10739)"); + let msg = result.unwrap_err().to_string(); + assert!(msg.contains("200 segments"), "error should name the count: {msg}"); + } } diff --git a/src/stdlib/set.rs b/src/stdlib/set.rs index 829353ec03..f54a4bfde6 100644 --- a/src/stdlib/set.rs +++ b/src/stdlib/set.rs @@ -1,9 +1,19 @@ use crate::compiler::prelude::*; use crate::path::{OwnedSegment, OwnedValuePath}; +const MAX_PATH_SEGMENTS: usize = 128; + fn set(path: Value, mut value: Value, data: Value) -> Resolved { let path = match path { Value::Array(segments) => { + if segments.len() > MAX_PATH_SEGMENTS { + return Err(format!( + "path has {} segments, max is {MAX_PATH_SEGMENTS}", + segments.len() + ) + .into()); + } + let mut insert = OwnedValuePath::root(); for segment in segments { @@ -186,4 +196,15 @@ mod tests { tdef: TypeDef::object(Collection::any()).fallible(), } ]; + + #[test] + fn test_path_length_limit_obe10739() { + // OBE-10739: attacker-controlled path with > MAX_PATH_SEGMENTS segments must be rejected. + // Without the fix this returns Ok and accepts arbitrary-depth writes. + let segments: Vec = (0..200).map(|i| Value::Bytes(format!("k{i}").into())).collect(); + let result = set(Value::Array(segments), Value::Object(ObjectMap::new()), Value::Null); + assert!(result.is_err(), "path with 200 segments must be rejected (OBE-10739)"); + let msg = result.unwrap_err().to_string(); + assert!(msg.contains("200 segments"), "error should name the count: {msg}"); + } } diff --git a/src/stdlib/starts_with.rs b/src/stdlib/starts_with.rs index d6e48abed8..988197bb50 100644 --- a/src/stdlib/starts_with.rs +++ b/src/stdlib/starts_with.rs @@ -23,6 +23,12 @@ impl Iterator for Chars<'_> { if width == 1 { self.pos += 1; Some(Ok(self.bytes[self.pos - 1] as char)) + } else if width == 0 || self.pos + width > self.bytes.len() { + // Invalid lead byte (width==0 for continuation/forbidden bytes) or truncated + // multi-byte sequence: yield the raw byte as an error and advance by one. + let byte = self.bytes[self.pos]; + self.pos += 1; + Some(Err(byte)) } else { let c = std::str::from_utf8(&self.bytes[self.pos..self.pos + width]); match c { @@ -31,8 +37,9 @@ impl Iterator for Chars<'_> { Some(Ok(chr.chars().next().unwrap())) } Err(_) => { + let byte = self.bytes[self.pos]; self.pos += 1; - Some(Err(self.bytes[self.pos])) + Some(Err(byte)) } } } @@ -165,6 +172,7 @@ impl FunctionExpression for StartsWithFn { #[cfg(test)] mod tests { use super::*; + use bytes::Bytes; test_function![ starts_with => StartsWith; @@ -269,5 +277,24 @@ mod tests { want: Ok(true), tdef: TypeDef::boolean().infallible(), } + + // OBE-10733: stray continuation byte (0x80) causes width==0 → panic without the fix + invalid_utf8_lead_byte_case_insensitive { + args: func_args![value: Value::Bytes(Bytes::from(vec![0x80u8, b'a', b'b', b'c'])), + substring: "abc", + case_sensitive: false + ], + want: Ok(false), + tdef: TypeDef::boolean().infallible(), + } + + invalid_utf8_truncated_multibyte { + args: func_args![value: Value::Bytes(Bytes::from(vec![0xc3u8])), + substring: "a", + case_sensitive: false + ], + want: Ok(false), + tdef: TypeDef::boolean().infallible(), + } ]; } diff --git a/src/stdlib/unflatten.rs b/src/stdlib/unflatten.rs index be082246f2..b79d383242 100644 --- a/src/stdlib/unflatten.rs +++ b/src/stdlib/unflatten.rs @@ -4,6 +4,8 @@ use crate::compiler::prelude::*; static DEFAULT_SEPARATOR: &str = "."; +const MAX_UNFLATTEN_DEPTH: usize = 128; + fn unflatten(value: Value, separator: Value, recursive: Value) -> Resolved { let separator = separator.try_bytes_utf8_lossy()?.into_owned(); let recursive = recursive.try_boolean()?; @@ -13,16 +15,21 @@ fn unflatten(value: Value, separator: Value, recursive: Value) -> Resolved { fn do_unflatten(value: Value, separator: &str, recursive: bool) -> Value { match value { - Value::Object(map) => do_unflatten_entries(map, separator, recursive).into(), + Value::Object(map) => do_unflatten_entries(map, separator, recursive, 0).into(), // Note that objects inside arrays are not unflattened _ => value, } } -fn do_unflatten_entries(entries: I, separator: &str, recursive: bool) -> ObjectMap +fn do_unflatten_entries(entries: I, separator: &str, recursive: bool, depth: usize) -> ObjectMap where I: IntoIterator, { + if depth >= MAX_UNFLATTEN_DEPTH { + // Stop splitting keys at the depth limit; collect remainder as literal keys. + return entries.into_iter().collect(); + } + let grouped = entries .into_iter() .map(|(key, value)| { @@ -72,7 +79,7 @@ where rest.map(|rest| (rest, value)) }) .collect::>(); - let result = do_unflatten_entries(new_entries, separator, recursive); + let result = do_unflatten_entries(new_entries, separator, recursive, depth + 1); (key, result.into()) }) .collect() @@ -83,7 +90,12 @@ where // and avoid doing recursive calls to `do_unflatten_entries` with a single entry every time fn do_unflatten_entry(entry: (KeyString, Value), separator: &str, recursive: bool) -> Value { let (key, value) = entry; - let keys = key.split(separator).map(Into::into).collect::>(); + // splitn caps the segment count at MAX_UNFLATTEN_DEPTH+1; the final piece retains + // any remaining separator characters as a literal key (OBE-10744). + let keys: Vec = key + .splitn(MAX_UNFLATTEN_DEPTH + 1, separator) + .map(Into::into) + .collect(); let mut result = if recursive { do_unflatten(value, separator, recursive) } else { @@ -413,4 +425,30 @@ mod test { tdef: TypeDef::object(Collection::any()), } ]; + + #[test] + fn test_depth_limit_obe10744() { + // OBE-10744: unflatten with deeply grouped entries must terminate at MAX_UNFLATTEN_DEPTH. + // Build two entries sharing a 200-level common prefix ("k0.k1...k199.x" and "...y"). + // Without the fix, do_unflatten_entries recurses 200 times and can stack overflow. + let prefix: Vec = (0..200).map(|i| format!("k{i}")).collect(); + let key1: KeyString = [prefix.join("."), "x".into()].join(".").into(); + let key2: KeyString = [prefix.join("."), "y".into()].join(".").into(); + let entries: Vec<(KeyString, Value)> = vec![(key1, Value::Integer(1)), (key2, Value::Integer(2))]; + // Must return without panicking and depth must be bounded. + let result = do_unflatten_entries(entries, ".", false, 0); + // Walk result to verify nesting depth is bounded. + let mut depth = 0usize; + let mut cur = Value::Object(result); + loop { + match cur { + Value::Object(ref m) if m.len() == 1 => { + cur = m.values().next().unwrap().clone(); + depth += 1; + assert!(depth <= MAX_UNFLATTEN_DEPTH + 1, "nesting depth {depth} exceeds cap (OBE-10744)"); + } + _ => break, + } + } + } } diff --git a/src/value/value/crud/insert.rs b/src/value/value/crud/insert.rs index 499905081b..4c3af856e0 100644 --- a/src/value/value/crud/insert.rs +++ b/src/value/value/crud/insert.rs @@ -26,10 +26,11 @@ pub fn insert<'a, T: ValueCollection>( if let Some(Value::Array(array)) = value.get_mut_value(key.borrow()) { insert(array, index, path_iter, insert_value) } else { + const MAX_ARRAY_CAPACITY: usize = 32_769; let capacity = if index >= 0 { - (index as usize) + 1 + ((index as usize) + 1).min(MAX_ARRAY_CAPACITY) } else { - (-index) as usize + ((-index) as usize).min(MAX_ARRAY_CAPACITY) }; let mut array = Vec::with_capacity(capacity); let prev_value = insert(&mut array, index, path_iter, insert_value); diff --git a/src/value/value/crud/mod.rs b/src/value/value/crud/mod.rs index 9883adc515..bc12625b3d 100644 --- a/src/value/value/crud/mod.rs +++ b/src/value/value/crud/mod.rs @@ -104,6 +104,10 @@ impl ValueCollection for Vec { } fn insert_value(&mut self, key: isize, value: Value) -> Option { + const MAX_ARRAY_INDEX: isize = 32_768; + if key > MAX_ARRAY_INDEX || key < -MAX_ARRAY_INDEX { + return None; + } if key >= 0 { if self.len() <= (key as usize) { while self.len() <= (key as usize) {