Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions src/compiler/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ pub struct Compiler<'a> {
// the error from the LHS)
fallible_expression_error: Option<CompilerError>,

/// Current expression nesting depth, incremented on each compile_expr entry.
depth: u32,

config: CompileConfig,
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -148,7 +152,27 @@ impl<'a> Compiler<'a> {
Some(exprs)
}

const MAX_EXPR_DEPTH: u32 = 128;

fn compile_expr(&mut self, node: Node<ast::Expr>, state: &mut TypeState) -> Option<Expr> {
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<ast::Expr>, state: &mut TypeState) -> Option<Expr> {
use ast::Expr::{
Abort, Assignment, Container, FunctionCall, IfStatement, Literal, Op, Query, Return,
Unary, Variable,
Expand Down Expand Up @@ -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)");
}
}
35 changes: 31 additions & 4 deletions src/compiler/value/arithmetic.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#![deny(clippy::arithmetic_side_effects)]

use std::ops::{Add, Mul, Rem};

use crate::compiler::{
value::{Kind, VrlValueConvert},
Expand Down Expand Up @@ -68,6 +67,33 @@ fn safe_sub(lhv: f64, rhv: f64) -> Option<Value> {
}
}

fn safe_add(lhv: f64, rhv: f64) -> Option<Value> {
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<Value> {
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<Value> {
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<Self, ValueError> {
Expand All @@ -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()
Expand Down Expand Up @@ -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)) => {
Expand Down Expand Up @@ -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()),
};
Expand Down
1 change: 1 addition & 0 deletions src/parser/lex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1290,6 +1290,7 @@ fn unescape_string_literal(mut s: &str) -> String {
b't' => '\t',
b'0' => '\0',
b'{' => '{',
b'}' => '}',
_ => unimplemented!("invalid escape"),
};

Expand Down
134 changes: 84 additions & 50 deletions src/parsing/ruby_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Value> {
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.
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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)");
}
}
Loading