From 0ef58efbc1ef275992a529366ff1133aee0dfe9d Mon Sep 17 00:00:00 2001 From: Aysha Afrah Ziya Date: Sun, 25 Oct 2026 22:55:23 +0530 Subject: [PATCH] bound the AST height of flat script expressions The nesting cap in ScriptParser::parseExpr counts parser recursion only. A flat chain such as 1+1+1+... is parsed by the loop without recursing, but it builds a left-deep AST that evaluate() and the node destructors walk recursively, one frame per operator, so a long Script attribute passes tree creation and overflows the stack on the first tick. Track the height of the expression being built and reject it past 1024 levels, so the evaluation depth no longer grows with the script length. --- src/script_parser.cpp | 41 ++++++++++++++++++++++++++++++++++++ tests/script_parser_test.cpp | 39 ++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/src/script_parser.cpp b/src/script_parser.cpp index 34f1d5e8a..7ffa67aa7 100644 --- a/src/script_parser.cpp +++ b/src/script_parser.cpp @@ -14,6 +14,7 @@ #include "behaviortree_cpp/scripting/operators.hpp" +#include #include namespace BT @@ -55,6 +56,15 @@ class ScriptParser // parser already applies in VerifyXML and recursivelyCreateSubtree. static constexpr int kMaxNestingDepth = 256; + // Height of the AST returned by the last parsePrefix() / parseExpr() call. + // A flat chain like "1+1+1+..." is parsed by the loop in parseExpr without + // recursing, but it builds a left-deep AST that evaluate() and the node + // destructors walk recursively, one frame per operator. Bounding the height + // bounds that recursion; it is larger than kMaxNestingDepth so that long + // flat expressions keep working. + int height_ = 0; + static constexpr int kMaxExpressionHeight = 1024; + // Binding power constants. Higher value = tighter binding. static constexpr int kAssignmentBP = 2; static constexpr int kTernaryBP = 4; @@ -165,18 +175,36 @@ class ScriptParser type == TokenType::StarEqual || type == TokenType::SlashEqual; } + // Height of a node whose children are a subtree of height lhs_height and the + // expression parsed last (height_). Throws if the AST gets too tall. + int combinedHeight(int lhs_height) const + { + const int height = std::max(lhs_height, height_) + 1; + if(height > kMaxExpressionHeight) + { + throw RuntimeError(StrCat("Parse error at position ", std::to_string(peek().pos), + ": expression is too long or too deeply nested (limit ", + std::to_string(kMaxExpressionHeight), " levels)")); + } + return height; + } + //--- Parsing --- /// Prefix: atoms and unary prefix operators Ast::expr_ptr parsePrefix() { const auto& tok = peek(); + // literals and identifiers are leaves; the cases below that parse a + // subexpression overwrite it + height_ = 1; // Unary minus if(tok.type == TokenType::Minus) { advance(); auto operand = parseExpr(kPrefixBP); + height_ = combinedHeight(0); return std::make_shared(Ast::ExprUnaryArithmetic::negate, std::move(operand)); } @@ -185,6 +213,7 @@ class ScriptParser { advance(); auto operand = parseExpr(kPrefixBP); + height_ = combinedHeight(0); return std::make_shared( Ast::ExprUnaryArithmetic::complement, std::move(operand)); } @@ -193,6 +222,7 @@ class ScriptParser { advance(); auto operand = parseExpr(kPrefixBP); + height_ = combinedHeight(0); return std::make_shared( Ast::ExprUnaryArithmetic::logical_not, std::move(operand)); } @@ -280,6 +310,7 @@ class ScriptParser } auto left = parsePrefix(); + int left_height = height_; while(true) { @@ -294,6 +325,7 @@ class ScriptParser if(isAssignment(tokType)) { left = parseAssignment(std::move(left)); + left_height = combinedHeight(left_height); break; } @@ -301,6 +333,7 @@ class ScriptParser if(tokType == TokenType::Question) { left = parseTernary(std::move(left)); + left_height = combinedHeight(left_height); break; } @@ -308,6 +341,7 @@ class ScriptParser if(isComparison(tokType)) { left = parseChainedComparison(std::move(left)); + left_height = combinedHeight(left_height); continue; } @@ -316,9 +350,11 @@ class ScriptParser // Right BP = LBP + 1 for left-associativity auto right = parseExpr(lbp + 1); left = makeBinary(std::move(left), opTok.type, std::move(right)); + left_height = combinedHeight(left_height); } --depth_; + height_ = left_height; return left; } @@ -358,8 +394,10 @@ class ScriptParser { advance(); // consume '?' auto thenExpr = parseExpr(0); // full expression inside + const int then_height = height_; expect(TokenType::Colon, "expected ':' in ternary expression"); auto elseExpr = parseExpr(kTernaryBP); + height_ = std::max(then_height, height_); return std::make_shared(std::move(condition), std::move(thenExpr), std::move(elseExpr)); } @@ -369,13 +407,16 @@ class ScriptParser auto node = std::make_shared(); node->operands.push_back(std::move(first)); + int max_height = 0; while(isComparison(peek().type)) { node->ops.push_back(mapComparisonOp(advance().type)); // Parse the next operand above comparison level // so that arithmetic binds tighter node->operands.push_back(parseExpr(kComparisonBP + 1)); + max_height = std::max(max_height, height_); } + height_ = max_height; return node; } diff --git a/tests/script_parser_test.cpp b/tests/script_parser_test.cpp index 9a4360eff..a9bc67b74 100644 --- a/tests/script_parser_test.cpp +++ b/tests/script_parser_test.cpp @@ -143,6 +143,45 @@ TEST(ParserTest, DeeplyNestedScriptIsRejected) EXPECT_TRUE(BT::ValidateScript("((((1))))")); } +TEST(ParserTest, LongFlatScriptIsRejected) +{ + // A flat chain like "1+1+1+..." is parsed without recursion, but it builds a + // left-deep AST that is evaluated (and destroyed) recursively, one stack frame + // per operator. 100k terms used to overflow the stack (SIGSEGV) when executed. + BT::Ast::Environment env = { BT::Blackboard::create(), {} }; + + std::string flat = "x := 1"; + for(int i = 0; i < 100000; ++i) + { + flat += "+1"; + } + EXPECT_FALSE(BT::ValidateScript(flat)); + EXPECT_FALSE(BT::ParseScriptAndExecute(env, flat).has_value()); + + // Nested chains add up as well: 100 levels of 20 operators each. + std::string nested_chain = "1"; + for(int i = 0; i < 100; ++i) + { + nested_chain = "(" + nested_chain; + for(int j = 0; j < 20; ++j) + { + nested_chain += "+1"; + } + nested_chain += ")"; + } + EXPECT_FALSE(BT::ValidateScript(nested_chain)); + + // Ordinary long expressions are still accepted and evaluated. + std::string wide = "1"; + for(int i = 0; i < 1000; ++i) + { + wide += "+1"; + } + auto result = BT::ParseScriptAndExecute(env, wide); + ASSERT_TRUE(result.has_value()); + EXPECT_EQ(result->cast(), 1001); +} + TEST(ParserTest, Equations) { BT::Ast::Environment environment = { BT::Blackboard::create(), {} };