diff --git a/lib/tests/tests/issues/obe_10727_float_nan_arithmetic.vrl b/lib/tests/tests/issues/obe_10727_float_nan_arithmetic.vrl new file mode 100644 index 0000000000..531004151f --- /dev/null +++ b/lib/tests/tests/issues/obe_10727_float_nan_arithmetic.vrl @@ -0,0 +1,13 @@ +# issue: OBE-10727 +# Overflowing multiplication produces a legitimate `inf` float, and multiplying that by zero +# yields NaN. `NotNan`'s `Mul` impl panicked on that, taking the process down. +# result: can't multiply type float by float + +x = 1000000.0 +x = x * x +x = x * x +x = x * x +x = x * x +x = x * x +x = x * x +x * 0.0 diff --git a/lib/tests/tests/issues/obe_10731_xml_comment_only_child.vrl b/lib/tests/tests/issues/obe_10731_xml_comment_only_child.vrl new file mode 100644 index 0000000000..5cabffe8f3 --- /dev/null +++ b/lib/tests/tests/issues/obe_10731_xml_comment_only_child.vrl @@ -0,0 +1,6 @@ +# issue: OBE-10731 +# An element whose only child is a comment took the single-child "flatten" path and handed the +# comment node to `process_node`, hitting `unreachable!("shouldn't be other XML nodes")`. +# result: { "a": {} } + +parse_xml!("") diff --git a/lib/tests/tests/issues/obe_10734_escaped_closing_brace.vrl b/lib/tests/tests/issues/obe_10734_escaped_closing_brace.vrl new file mode 100644 index 0000000000..3a4adb0262 --- /dev/null +++ b/lib/tests/tests/issues/obe_10734_escaped_closing_brace.vrl @@ -0,0 +1,7 @@ +# issue: OBE-10734 +# The lexer accepted `\}` as an escape code, but `unescape_string_literal` had no matching arm +# and fell through to `unimplemented!("invalid escape")` — a one-character program crashed the +# compiler. +# result: "}" + +"\}" diff --git a/lib/tests/tests/issues/obe_10735_array_index_cap.vrl b/lib/tests/tests/issues/obe_10735_array_index_cap.vrl new file mode 100644 index 0000000000..ff26e9f39f --- /dev/null +++ b/lib/tests/tests/issues/obe_10735_array_index_cap.vrl @@ -0,0 +1,12 @@ +# issue: OBE-10735 +# Assigning to a large array index padded the array with `Value::Null` up to that index with no +# cap, so an event-controlled index could exhaust memory. Indices beyond ±32768 are now ignored. +# result: [0, 32769] + +capped = [] +capped[40000] = 1 + +allowed = [] +allowed[32768] = 1 + +[length(capped), length(allowed)] diff --git a/src/compiler/value/arithmetic.rs b/src/compiler/value/arithmetic.rs index 80b63b30bc..3d2ff351a1 100644 --- a/src/compiler/value/arithmetic.rs +++ b/src/compiler/value/arithmetic.rs @@ -1,7 +1,5 @@ #![deny(clippy::arithmetic_side_effects)] -use std::ops::{Add, Mul, Rem}; - use crate::compiler::{ value::{Kind, VrlValueConvert}, ExpressionError, @@ -68,6 +66,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 +115,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 +159,7 @@ 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 +255,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()), }; @@ -342,3 +367,50 @@ impl VrlValueArithmetic for Value { } } } + +#[cfg(test)] +mod tests { + use ordered_float::NotNan; + + use super::*; + + // `NotNan` permits infinities, so a float operand can legitimately be ±∞ (e.g. produced by + // an overflowing multiplication, or by `parse_json` on an out-of-range literal). Operations + // whose result is NaN used to reach `NotNan`'s `Add`/`Mul`/`Rem` impls, which panic. + // See OBE-10727. + fn float(v: f64) -> Value { + Value::Float(NotNan::new(v).expect("test operand is not NaN")) + } + + #[test] + fn multiplying_infinity_by_zero_returns_an_error_instead_of_panicking() { + assert!(float(f64::INFINITY).try_mul(float(0.0)).is_err()); + assert!(float(0.0).try_mul(float(f64::NEG_INFINITY)).is_err()); + } + + #[test] + fn adding_opposite_infinities_returns_an_error_instead_of_panicking() { + assert!(float(f64::INFINITY) + .try_add(float(f64::NEG_INFINITY)) + .is_err()); + } + + #[test] + fn taking_the_remainder_of_infinity_returns_an_error_instead_of_panicking() { + assert!(float(f64::INFINITY).try_rem(float(f64::INFINITY)).is_err()); + assert!(float(f64::INFINITY).try_rem(float(2.0)).is_err()); + } + + #[test] + fn an_infinite_result_is_still_a_valid_value() { + // Only NaN results are rejected — overflow to ±∞ must keep working. + assert_eq!( + float(f64::MAX).try_mul(float(10.0)), + Ok(float(f64::INFINITY)) + ); + assert_eq!( + float(f64::INFINITY).try_add(float(1.0)), + Ok(float(f64::INFINITY)) + ); + } +} diff --git a/src/parser/lex.rs b/src/parser/lex.rs index 620bbd7cd2..9697cad744 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"), }; @@ -2146,6 +2147,14 @@ mod test { ); } + // OBE-10734: the lexer's `escape_code` accepted `\}`, but `unescape_string_literal` had no + // matching arm and fell through to `unimplemented!("invalid escape")`. + #[test] + fn unescape_escaped_closing_brace() { + assert_eq!("}", StringLiteralToken(r"\}").unescape()); + assert_eq!("{}", StringLiteralToken(r"\{\}").unescape()); + } + #[test] fn function_closure_no_arg() { test( diff --git a/src/parsing/xml.rs b/src/parsing/xml.rs index 1bbfd1dc15..f28fdf33f7 100644 --- a/src/parsing/xml.rs +++ b/src/parsing/xml.rs @@ -162,22 +162,23 @@ fn process_node(node: Node, config: &ParseXmlConfig) -> Value { _ => 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), + ); + + Value::Object(map) + } + Some(node) => process_node(node, config), + // Only child is a comment or PI — treat as empty element. + None => Value::Object(recurse(node)), } } // For 2+ nodes, expand. 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..a72bcf1f7b 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 { @@ -136,7 +143,7 @@ impl Function for FormatNumber { fn examples(&self) -> &'static [Example] { &[Example { title: "format number", - source: r#"format_number(4672.4, decimal_separator: ",", grouping_separator: "_")"#, + source: r#"format_number!(4672.4, decimal_separator: ",", grouping_separator: "_")"#, result: Ok("4_672,4"), }] } @@ -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..c50ca7fbd9 100644 --- a/src/stdlib/parse_xml.rs +++ b/src/stdlib/parse_xml.rs @@ -479,6 +479,27 @@ mod tests { want: Ok(value!({ "root": { "a": { "a1": "test" }, "b" : "test2" } })), tdef: type_def(), } + + // OBE-10731: an element whose only child is a comment or processing instruction took + // the single-child "flatten" path, handing a non-element/non-text node to + // `process_node`, which hits `unreachable!("shouldn't be other XML nodes")`. + only_child_is_a_comment { + args: func_args![ value: ""], + want: Ok(value!({ "a": {} })), + tdef: type_def(), + } + + only_child_is_a_processing_instruction { + args: func_args![ value: ""], + want: Ok(value!({ "a": {} })), + tdef: type_def(), + } + + comment_alongside_an_element_child_is_skipped { + args: func_args![ value: "x"], + want: Ok(value!({ "a": { "b": "x" } })), + tdef: type_def(), + } ]; #[test] 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/value/value/crud/insert.rs b/src/value/value/crud/insert.rs index 499905081b..23ff451132 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); @@ -77,6 +78,32 @@ mod test { assert_eq!(value, expected); } + // OBE-10735: `insert_value` padded the array with `Value::Null` up to an arbitrary index, + // and `Vec::with_capacity(index + 1)` allocated for it up front — an event-controlled path + // index was enough to exhaust memory. + #[test] + fn test_insert_beyond_max_array_index_is_rejected() { + let mut value = Value::Null; + assert_eq!(value.insert("[40000]", 1), None); + assert_eq!(value, Value::from(json!([]))); + } + + #[test] + fn test_insert_beyond_max_negative_array_index_is_rejected() { + let mut value = Value::Null; + assert_eq!(value.insert("[-40000]", 1), None); + assert_eq!(value, Value::from(json!([]))); + } + + #[test] + fn test_insert_at_max_array_index_is_allowed() { + let mut value = Value::Null; + assert_eq!(value.insert("[32768]", 1), None); + let array = value.as_array().expect("expected an array"); + assert_eq!(array.len(), 32769); + assert_eq!(array[32768], Value::Integer(1)); + } + #[test] fn test_insert_negative_index() { let mut value = Value::Null; diff --git a/src/value/value/crud/mod.rs b/src/value/value/crud/mod.rs index 9883adc515..2f2f02ef06 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 !(-MAX_ARRAY_INDEX..=MAX_ARRAY_INDEX).contains(&key) { + return None; + } if key >= 0 { if self.len() <= (key as usize) { while self.len() <= (key as usize) {