diff --git a/src/parser/mod.rs b/src/parser/mod.rs index b2b3f42bb..e60ef1011 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -442,6 +442,24 @@ impl<'a> Parser<'a> { /// # } /// ``` /// + /// The limit is only meaningful where it is actually charged. Every cycle in + /// the parser's call graph must pass through a `try_decrease` site, or input + /// driving that cycle recurses with no accounting and the limit is inert for + /// it. The counted set is deliberately the minimum that cuts every cycle: + /// [`Self::parse_statement`], [`Self::parse_subexpr`], [`Self::parse_query`], + /// [`Self::parse_table_factor`], `parse_data_type_helper`, [`Self::parse_interval`], + /// `parse_pattern`, [`Self::parse_json_table_column_def`], + /// `parse_key_value_options` and `parse_joins`. One further cycle is + /// intentionally uncounted because it cannot recur on input — see the comment + /// on `parse_remaining_set_exprs`. Adding a recursive path that reaches none + /// of these reintroduces the gap. + /// + /// Note also that reaching the limit itself costs stack: the deepest chain + /// (`parse_prefix` -> `parse_cast_expr` -> `parse_expr` -> `parse_subexpr`) runs + /// tens of KB per level in an unoptimised build, so the default of 50 wants a + /// few MB of headroom to deny cleanly. Callers running on small stacks should + /// lower the limit rather than rely on the default. + /// /// Note: when "recursive-protection" feature is enabled, this crate uses additional stack overflow protection // for some of its recursive methods. See [`recursive::recursive`] for more information. pub fn with_recursion_limit(mut self, recursion_limit: usize) -> Self { @@ -1895,6 +1913,18 @@ impl<'a> Parser<'a> { Err(e) => { self.failed_reserved_word_prefix_positions .insert(next_token_index, (&e).into()); + // A recursion-limit error is not "this word meant + // something else" -- it is "we ran out of depth". Retrying + // the same span under a second interpretation re-descends + // the same input, once per level, which turns a bounded + // denial into exponential backtracking. It also silently + // changes the AST when the retry happens to succeed, so the + // same SQL parses differently at different recursion limits. + // `maybe_parse` re-raises this error precisely so callers + // can propagate it; do not discard it here. + if matches!(e, ParserError::RecursionLimitExceeded) { + return Err(e); + } if !self.dialect.is_reserved_for_identifier(w.keyword) { if let Ok(Some(expr)) = self.maybe_parse(|parser| { parser.parse_expr_prefix_by_unreserved_word(&w, span) @@ -12766,6 +12796,12 @@ impl<'a> Parser<'a> { fn parse_data_type_helper( &mut self, ) -> Result<(DataType, MatchedTrailingBracket), ParserError> { + // The guard belongs HERE and not on the public `parse_data_type`: the + // recursive type constructors are not all routed through it. `ARRAY<..>` + // and `STRUCT<..>` recurse into `parse_data_type_helper` directly (to + // thread `MatchedTrailingBracket` through), so a guard on + // `parse_data_type` alone would leave the angle-bracket forms uncounted. + let _guard = self.recursion_counter.try_decrease()?; let dialect = self.dialect; self.advance_token(); let next_token = self.get_current_token(); @@ -15093,6 +15129,15 @@ impl<'a> Parser<'a> { /// Parse any extra set expressions that may be present in a query body /// /// (this is its own function to reduce required stack size in debug builds) + /// + /// Not counted, deliberately. This forms a cycle with [`Self::parse_query_body`], + /// but it cannot recur without bound on attacker-shaped input: the left side is + /// consumed by the `loop` below (so `A UNION B UNION C ..` is iterative, at any + /// length), and the right side only recurses when the next operator binds more + /// tightly. There are exactly two set-operation precedences (10 for + /// UNION/EXCEPT/MINUS, 20 for INTERSECT) and the recursive call passes the + /// higher one, so `precedence >= next_precedence` breaks immediately at the + /// next level. Depth is bounded by the precedence ladder at 2, not by input. fn parse_remaining_set_exprs( &mut self, mut expr: SetExpr, @@ -16226,6 +16271,16 @@ impl<'a> Parser<'a> { } fn parse_joins(&mut self) -> Result, ParserError> { + // Self-recursive through the parens-less nested-join branch below, on + // dialects where `supports_left_associative_joins_without_parens` is + // false (Snowflake). `SELECT * FROM t JOIN t JOIN t ..` drives it. + // + // It is tempting to argue this is already charged because reaching the + // recursive call requires a `parse_table_factor`, which takes a guard. + // That is wrong: `DepthGuard` releases on drop, and `parse_table_factor` + // has already returned by the time we recurse, so its guard is not live + // and nothing accumulates. Count it here. + let _guard = self.recursion_counter.try_decrease()?; let mut joins = vec![]; loop { let global = self.parse_keyword(Keyword::GLOBAL); @@ -17348,6 +17403,11 @@ impl<'a> Parser<'a> { } fn parse_pattern(&mut self) -> Result { + // MATCH_RECOGNIZE patterns are their own recursive-descent grammar + // (pattern -> concat -> repetition -> base -> pattern) reached from + // `parse_match_recognize`. Nothing on that cycle is counted, and nested + // parentheses inside PATTERN(..) drive it. + let _guard = self.recursion_counter.try_decrease()?; let pattern = self.parse_concat_pattern()?; if self.consume_token(&Token::Pipe) { match self.parse_pattern()? { @@ -17414,6 +17474,9 @@ impl<'a> Parser<'a> { /// Parses MySQL's JSON_TABLE column definition. /// For example: `id INT EXISTS PATH '$' DEFAULT '0' ON EMPTY ERROR ON ERROR` pub fn parse_json_table_column_def(&mut self) -> Result { + // `NESTED PATH '$' COLUMNS(..)` nests column definitions inside column + // definitions, so this function is directly self-recursive on input. + let _guard = self.recursion_counter.try_decrease()?; if self.parse_keyword(Keyword::NESTED) { let _has_path_keyword = self.parse_keyword(Keyword::PATH); let path = self.parse_value()?; @@ -21083,6 +21146,10 @@ impl<'a> Parser<'a> { parenthesized: bool, end_words: &[Keyword], ) -> Result { + // Option values may themselves be parenthesised option lists + // (`parse_key_value_option` calls back into this function), so nesting + // in e.g. Snowflake's `COPY_OPTIONS=(a=(b=(..)))` recurses on input. + let _guard = self.recursion_counter.try_decrease()?; let mut options: Vec = Vec::new(); let mut delimiter = KeyValueOptionsDelimiter::Space; if parenthesized { diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 0800bc41f..f642859ce 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -11419,6 +11419,274 @@ fn parse_deeply_nested_subquery_expr_hits_recursion_limits() { assert_eq!(res, Err(ParserError::RecursionLimitExceeded)); } +/// Parse `sql` on a thread with a bounded stack and a hard timeout. +/// +/// These shapes are bounded by *remaining stack*, not by input size, so an +/// unguarded recursion does not fail — it wedges. Running the parse on its own +/// thread with a receive timeout means a regression in the depth guard fails the +/// test loudly and in bounded time instead of hanging the test process. +/// +/// A successful parse is `mem::forget`-ed rather than dropped: the derived `Drop` +/// on the AST is itself recursive, so dropping a deep tree performs exactly the +/// stack descent this test exists to prevent. +/// +/// The stack is deliberately *generous* rather than tight. What is under test is +/// the depth contract, and reaching `DEFAULT_REMAINING_DEPTH` at all costs real +/// stack: the `parse_prefix -> parse_cast_expr -> parse_expr -> parse_subexpr` +/// chain runs ~85 KB per level in an unoptimised build, so a full 50 levels needs +/// roughly 5 MB before the guard can fire. A tighter stack would make this test +/// fail on the debug frame budget instead of on the property it asserts. The +/// loudness comes from the assertion and the timeout, not from starving the stack. +fn parse_depth_bounded(name: &str, sql: String) -> Result<(), ParserError> { + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::Builder::new() + .stack_size(16 * 1024 * 1024) + .spawn(move || { + let dialect = GenericDialect {}; + let result = Parser::new(&dialect) + .try_with_sql(&sql) + .expect("tokenize to work") + .parse_statements(); + let _ = tx.send(match result { + Ok(ast) => { + std::mem::forget(ast); + Ok(()) + } + Err(e) => Err(e), + }); + }) + .expect("spawn to work"); + + rx.recv_timeout(std::time::Duration::from_secs(20)) + .unwrap_or_else(|_| { + panic!("{name}: parser did not return within 20s; the depth guard has regressed") + }) +} + +/// Every recursive-descent shape that reaches an uncounted cycle must deny with +/// `RecursionLimitExceeded`, not run to stack exhaustion. +/// +/// Regression test for the audit in zuru-federated-query#606. Each entry drives a +/// distinct cycle; see the comments at the corresponding `try_decrease` sites. +#[test] +fn deeply_nested_recursive_shapes_hit_recursion_limits() { + // depth far past DEFAULT_REMAINING_DEPTH (50), but only a few KB of SQL + let n = 2000; + let cases: Vec<(&str, String)> = vec![ + // --- recursive DataType constructors, via CAST --- + ( + "Nullable", + format!( + "SELECT CAST(1 AS {}INT{})", + "Nullable(".repeat(n), + ")".repeat(n) + ), + ), + ( + "LowCardinality", + format!( + "SELECT CAST(1 AS {}INT{})", + "LowCardinality(".repeat(n), + ")".repeat(n) + ), + ), + ( + "Map", + format!( + "SELECT CAST(1 AS {}INT{})", + "Map(INT, ".repeat(n), + ")".repeat(n) + ), + ), + ( + "Tuple", + format!( + "SELECT CAST(1 AS {}INT{})", + "Tuple(a ".repeat(n), + ")".repeat(n) + ), + ), + ( + "Nested", + format!( + "SELECT CAST(1 AS {}INT{})", + "NESTED(a ".repeat(n), + ")".repeat(n) + ), + ), + // Angle-bracket forms recurse into `parse_data_type_helper` *directly*, + // bypassing `parse_data_type`. A guard placed on `parse_data_type` alone + // would not catch these two — which is why the guard is on the helper. + ( + "ARRAY<>", + format!( + "SELECT CAST(1 AS {}INT{})", + "ARRAY<".repeat(n), + ">".repeat(n) + ), + ), + ( + "STRUCT<>", + format!( + "SELECT CAST(1 AS {}INT{})", + "STRUCT".repeat(n) + ), + ), + // --- the same DataType cycle reached from non-CAST positions --- + ( + "TRY_CAST", + format!( + "SELECT TRY_CAST(1 AS {}INT{})", + "Nullable(".repeat(n), + ")".repeat(n) + ), + ), + ( + "::cast", + format!("SELECT 1::{}INT{}", "Nullable(".repeat(n), ")".repeat(n)), + ), + ( + "column def", + format!( + "CREATE TABLE t (c {}INT{})", + "Nullable(".repeat(n), + ")".repeat(n) + ), + ), + ( + "RETURNS TABLE", + format!( + "CREATE FUNCTION f() RETURNS TABLE (c {}INT{}) AS 'x'", + "Nullable(".repeat(n), + ")".repeat(n) + ), + ), + // --- cycles outside the DataType family --- + ( + "INTERVAL prefix chain", + format!("SELECT {}'1'", "INTERVAL ".repeat(n)), + ), + ( + "MATCH_RECOGNIZE PATTERN", + format!( + "SELECT * FROM t MATCH_RECOGNIZE(PATTERN ({}a{}) DEFINE a AS true)", + "(".repeat(n), + ")".repeat(n) + ), + ), + ( + "JSON_TABLE NESTED COLUMNS", + format!( + "SELECT * FROM JSON_TABLE('{{}}', '$' COLUMNS({}a INT PATH '$'{}))", + "NESTED PATH '$' COLUMNS(".repeat(n), + ")".repeat(n) + ), + ), + // --- shapes that reach a *counted* site, but whose recursion-limit error + // --- was previously discarded and retried in `parse_prefix` + ( + "nested CAST", + format!("SELECT {}1{}", "CAST(".repeat(n), " AS INT)".repeat(n)), + ), + ( + "nested CASE", + format!( + "SELECT {}1{}", + "CASE WHEN true THEN ".repeat(n), + " END".repeat(n) + ), + ), + ]; + + for (name, sql) in cases { + let res = parse_depth_bounded(name, sql); + assert_eq!( + Err(ParserError::RecursionLimitExceeded), + res, + "{name}: expected a clean recursion-limit denial" + ); + } +} + +/// The depth guard must not be so eager that it rejects ordinary SQL, and must +/// not convert the *iterative* deep shapes into denials. +#[test] +fn depth_guard_does_not_deny_shapes_that_are_not_recursive() { + // `INT[][]..[]` is assembled by a loop in `parse_data_type_helper`, not by + // recursive descent, so it must still parse at a depth no recursion could reach. + let sql = format!("SELECT CAST(1 AS INT{})", "[]".repeat(5000)); + assert_eq!( + Ok(()), + parse_depth_bounded("sq", sql), + "square-bracket array form" + ); + + // A long UNION chain is left-associative and parsed iteratively. + let sql = format!("SELECT 1{}", " UNION SELECT 1".repeat(5000)); + assert_eq!( + Ok(()), + parse_depth_bounded("union", sql), + "set-operation chain" + ); + + // Ordinary nested types stay well inside the budget. + let sql = "SELECT CAST(1 AS ARRAY>)".to_string(); + assert_eq!( + Ok(()), + parse_depth_bounded("ordinary", sql), + "ordinary nested type" + ); +} + +/// The recursion limit must change *whether* a statement parses, never *how* it +/// parses. Before the fix, `parse_prefix` discarded `RecursionLimitExceeded` and +/// retried the same span under a different interpretation, so the same SQL +/// produced structurally different ASTs at different limits — silently, as `Ok`. +#[test] +fn recursion_limit_does_not_change_the_parsed_ast() { + let dialect = GenericDialect {}; + let sql = format!( + "SELECT {}1{}", + "CASE WHEN true THEN ".repeat(6), + " END".repeat(6) + ); + + let mut parsed = None; + for limit in [20usize, 40, 60, 100] { + let res = Parser::new(&dialect) + .with_recursion_limit(limit) + .try_with_sql(&sql) + .expect("tokenize to work") + .parse_statements(); + + match res { + // Denial is a legitimate outcome at a tight limit. + Err(ParserError::RecursionLimitExceeded) => {} + Err(e) => panic!("limit {limit}: unexpected error {e}"), + Ok(ast) => { + let rendered = ast[0].to_string(); + if let Some(prev) = &parsed { + assert_eq!( + prev, &rendered, + "limit {limit} produced a different AST for identical SQL" + ); + } else { + parsed = Some(rendered); + } + } + } + } + + // Sanity: at least one limit must have been generous enough to succeed, and + // the successful parse must round-trip to the input. + assert_eq!( + Some(sql), + parsed, + "no limit parsed the statement successfully" + ); +} + #[test] fn parse_deeply_nested_interval_hits_recursion_limits() { let dialect = GenericDialect {}; diff --git a/tests/sqlparser_snowflake.rs b/tests/sqlparser_snowflake.rs index 059560dcc..dc3f45fae 100644 --- a/tests/sqlparser_snowflake.rs +++ b/tests/sqlparser_snowflake.rs @@ -3440,6 +3440,79 @@ fn test_parentheses_overflow() { assert_eq!(parsed.err(), Some(ParserError::RecursionLimitExceeded)); } +/// Nested parenthesised option lists recurse through +/// `parse_key_value_options` <-> `parse_key_value_option`, a cycle that reaches +/// no depth guard before this was counted. Regression test for the audit in +/// zuru-federated-query#606. +/// +/// Runs on its own thread with a timeout so a regression fails loudly and in +/// bounded time rather than wedging the test process — an uncounted recursion +/// here does not error, it exhausts the stack. +#[test] +fn test_nested_key_value_options_hit_recursion_limits() { + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::Builder::new() + .stack_size(16 * 1024 * 1024) + .spawn(move || { + let n = 2000; + let sql = format!( + "CREATE STAGE s COPY_OPTIONS=({}b=1{})", + "a=(".repeat(n), + ")".repeat(n) + ); + let parsed = snowflake().parse_sql_statements(&sql); + let _ = tx.send(match parsed { + Ok(ast) => { + // #602: the derived Drop is recursive; do not descend it here. + std::mem::forget(ast); + None + } + Err(e) => Some(e), + }); + }) + .expect("spawn to work"); + + let err = rx + .recv_timeout(std::time::Duration::from_secs(20)) + .expect("parser did not return within 20s; the depth guard has regressed"); + + assert_eq!(Some(ParserError::RecursionLimitExceeded), err); +} + +/// Snowflake sets `supports_left_associative_joins_without_parens` to false, so +/// `t JOIN t JOIN t ..` takes the parens-less nested-join branch, where +/// `parse_joins` calls itself once per JOIN. `parse_table_factor`'s guard does +/// not cover this: it has already returned, and `DepthGuard` releases on drop. +/// +/// Regression test for the audit in zuru-federated-query#606. +#[test] +fn test_parens_less_nested_joins_hit_recursion_limits() { + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::Builder::new() + .stack_size(16 * 1024 * 1024) + .spawn(move || { + let sql = format!("SELECT * FROM t{}", " JOIN t".repeat(2000)); + let parsed = snowflake().parse_sql_statements(&sql); + let _ = tx.send(match parsed { + Ok(ast) => { + std::mem::forget(ast); + None + } + Err(e) => Some(e), + }); + }) + .expect("spawn to work"); + + let err = rx + .recv_timeout(std::time::Duration::from_secs(20)) + .expect("parser did not return within 20s; the depth guard has regressed"); + + assert_eq!(Some(ParserError::RecursionLimitExceeded), err); + + // An ordinary join chain must still parse. + snowflake().verified_stmt("SELECT * FROM t JOIN t2 ON true JOIN t3 ON true"); +} + #[test] fn test_show_databases() { snowflake().verified_stmt("SHOW DATABASES");