diff --git a/parser/internal/options.h b/parser/internal/options.h index 4066d2619..cca9670a5 100644 --- a/parser/internal/options.h +++ b/parser/internal/options.h @@ -21,6 +21,7 @@ inline constexpr int kDefaultErrorRecoveryLimit = 12; inline constexpr int kDefaultMaxRecursionDepth = 32; inline constexpr int kExpressionSizeCodepointLimit = 100'000; inline constexpr int kDefaultErrorRecoveryTokenLookaheadLimit = 512; +inline constexpr int kDefaultExpressionNodeLimit = 100'000; inline constexpr bool kDefaultAddMacroCalls = false; } // namespace cel::parser_internal diff --git a/parser/internal/pratt_parser_worker.cc b/parser/internal/pratt_parser_worker.cc index c502796f5..f9efca03c 100644 --- a/parser/internal/pratt_parser_worker.cc +++ b/parser/internal/pratt_parser_worker.cc @@ -192,14 +192,24 @@ void ParserWorker::SynchronizeOnDelimiter() { NextToken(); } } + int64_t ParserWorker::NextId(int32_t position) { int64_t id = next_id_++; + if (id > options_.expression_node_limit) { + ReportError(position, "expression node limit exceeded"); + } if (position >= 0) { positions_.insert({id, position}); } return id; } +int64_t ParserWorker::NextId() { return NextId(-1); } + +bool ParserWorker::NodeLimitExceeded() { + return next_id_ > options_.expression_node_limit; +} + int64_t ParserWorker::CopyId(int64_t id) { if (id == 0) { return 0; diff --git a/parser/internal/pratt_parser_worker.h b/parser/internal/pratt_parser_worker.h index b022ae9ee..2e8534e78 100644 --- a/parser/internal/pratt_parser_worker.h +++ b/parser/internal/pratt_parser_worker.h @@ -76,7 +76,8 @@ class ParserWorker { // ID and Position tracking int64_t NextId(int32_t position); int64_t NextId(const Token& token) { return NextId(token.start); } - int64_t NextId() { return next_id_++; } + int64_t NextId(); + bool NodeLimitExceeded(); int64_t CopyId(int64_t id); void EraseId(int64_t id); @@ -1050,6 +1051,11 @@ std::optional PrattParserWorker::TryExpandMacro( if (!expander) { return std::nullopt; } + if (NodeLimitExceeded()) { + ReportError(expr_id, + "could not expand macro: expression node limit exceeded"); + return std::nullopt; + } std::vector macro_args; ExprNode macro_target; diff --git a/parser/options.h b/parser/options.h index eca41810a..eb9d2eeea 100644 --- a/parser/options.h +++ b/parser/options.h @@ -42,6 +42,14 @@ struct ParserOptions final { int error_recovery_token_lookahead_limit = ::cel::parser_internal::kDefaultErrorRecoveryTokenLookaheadLimit; + // Limit on the number of expression nodes in the abstract syntax tree for the + // expression. This prevents cases where macro expansion results in an AST + // that is larger than expected from the source expression. Once exceeded, + // the parser will record an error and stop expanding macros but continue + // parsing to report other errors. + int expression_node_limit = + ::cel::parser_internal::kDefaultExpressionNodeLimit; + // Add macro calls to macro_calls list in source_info. bool add_macro_calls = ::cel::parser_internal::kDefaultAddMacroCalls; diff --git a/parser/parser.cc b/parser/parser.cc index 6a66ffc9a..9bc471fdf 100644 --- a/parser/parser.cc +++ b/parser/parser.cc @@ -165,8 +165,9 @@ SourceRange SourceRangeFromParserRuleContext( class ParserMacroExprFactory final : public MacroExprFactory { public: - explicit ParserMacroExprFactory(const cel::Source& source) - : source_(source) {} + explicit ParserMacroExprFactory(const cel::Source& source, + int expression_node_limit) + : source_(source), expression_node_limit_(expression_node_limit) {} void BeginMacro(SourceRange macro_position) { macro_position_ = macro_position; @@ -203,12 +204,18 @@ class ParserMacroExprFactory final : public MacroExprFactory { int64_t NextId(const SourceRange& range) { auto id = expr_id_++; + if (id > expression_node_limit_ && !node_limit_exceeded_) { + node_limit_exceeded_ = true; + ReportError(range, "expression node limit exceeded"); + } if (range.begin != -1 || range.end != -1) { positions_.insert(std::pair{id, range}); } return id; } + bool is_node_limit_exceeded() const { return node_limit_exceeded_; } + bool HasErrors() const { return error_count_ != 0; } std::vector CollectIssues() { @@ -409,6 +416,8 @@ class ParserMacroExprFactory final : public MacroExprFactory { std::vector errors_; size_t error_count_ = 0; const Source& source_; + int expression_node_limit_; + bool node_limit_exceeded_ = false; SourceRange macro_position_; }; @@ -623,13 +632,14 @@ class ParserVisitor final : public CelBaseVisitor, public antlr4::BaseErrorListener { public: ParserVisitor(const cel::Source& source, int max_recursion_depth, + int max_expression_node_count, const cel::MacroRegistry& macro_registry, bool add_macro_calls = false, bool enable_optional_syntax = false, bool enable_quoted_identifiers = false, bool enable_variadic_logical_operators = false) : source_(source), - factory_(source_), + factory_(source_, max_expression_node_count), macro_registry_(macro_registry), recursion_depth_(0), max_recursion_depth_(max_recursion_depth), @@ -1227,6 +1237,8 @@ std::vector ParserVisitor::visitList( if (!enable_optional_syntax_ && expr_ctx->opt != nullptr) { factory_.ReportError(SourceRangeFromParserRuleContext(ctx), "unsupported syntax '?'"); + // Still generate an ID to detect node limit exceeded. + factory_.NextId(SourceRangeFromParserRuleContext(ctx)); rv.push_back(factory_.NewListElement(factory_.NewUnspecified(0), false)); continue; } @@ -1298,6 +1310,9 @@ std::vector ParserVisitor::visitEntries( if (!enable_optional_syntax_ && ctx->keys[i]->opt) { factory_.ReportError(SourceRangeFromParserRuleContext(ctx), "unsupported syntax '?'"); + // Still generate an ID to detect node limit exceeded. + factory_.NextId(SourceRangeFromParserRuleContext(ctx)); + factory_.NextId(SourceRangeFromParserRuleContext(ctx)); res.push_back(factory_.NewMapEntry(0, factory_.NewUnspecified(0), factory_.NewUnspecified(0), false)); continue; @@ -1461,29 +1476,34 @@ std::vector ParserVisitor::CollectIssues() { Expr ParserVisitor::GlobalCallOrMacroImpl(int64_t expr_id, absl::string_view function, std::vector args) { - if (auto macro = macro_registry_.FindMacro(function, args.size(), false); - macro) { - std::vector macro_args; - if (add_macro_calls_) { - macro_args.reserve(args.size()); - for (const auto& arg : args) { - macro_args.push_back(factory_.BuildMacroCallArg(arg)); - } + auto macro = macro_registry_.FindMacro(function, args.size(), false); + if (!macro) { + return factory_.NewCall(expr_id, function, std::move(args)); + } + if (factory_.is_node_limit_exceeded()) { + return factory_.ReportError( + factory_.GetSourceRange(expr_id), + "could not expand macro: expression node limit exceeded"); + } + std::vector macro_args; + if (add_macro_calls_) { + macro_args.reserve(args.size()); + for (const auto& arg : args) { + macro_args.push_back(factory_.BuildMacroCallArg(arg)); } - factory_.BeginMacro(factory_.GetSourceRange(expr_id)); - auto expr = macro->Expand(factory_, std::nullopt, absl::MakeSpan(args)); - factory_.EndMacro(); - if (expr) { - if (add_macro_calls_) { - factory_.AddMacroCall(expr->id(), function, std::nullopt, - std::move(macro_args)); - } - // We did not end up using `expr_id`. Delete metadata. - factory_.EraseId(expr_id); - return std::move(*expr); + } + factory_.BeginMacro(factory_.GetSourceRange(expr_id)); + auto expr = macro->Expand(factory_, std::nullopt, absl::MakeSpan(args)); + factory_.EndMacro(); + if (expr) { + if (add_macro_calls_) { + factory_.AddMacroCall(expr->id(), function, std::nullopt, + std::move(macro_args)); } + // We did not end up using `expr_id`. Delete metadata. + factory_.EraseId(expr_id); + return std::move(*expr); } - return factory_.NewCall(expr_id, function, std::move(args)); } @@ -1491,30 +1511,39 @@ Expr ParserVisitor::ReceiverCallOrMacroImpl(int64_t expr_id, absl::string_view function, Expr target, std::vector args) { - if (auto macro = macro_registry_.FindMacro(function, args.size(), true); - macro) { - Expr macro_target; - std::vector macro_args; - if (add_macro_calls_) { - macro_args.reserve(args.size()); - macro_target = factory_.BuildMacroCallArg(target); - for (const auto& arg : args) { - macro_args.push_back(factory_.BuildMacroCallArg(arg)); - } + auto macro = macro_registry_.FindMacro(function, args.size(), true); + if (!macro) { + return factory_.NewMemberCall(expr_id, function, std::move(target), + std::move(args)); + } + if (factory_.is_node_limit_exceeded()) { + return factory_.ReportError( + factory_.GetSourceRange(expr_id), + "could not expand macro: expression node limit exceeded"); + } + + Expr macro_target; + std::vector macro_args; + if (add_macro_calls_) { + macro_args.reserve(args.size()); + macro_target = factory_.BuildMacroCallArg(target); + for (const auto& arg : args) { + macro_args.push_back(factory_.BuildMacroCallArg(arg)); } - factory_.BeginMacro(factory_.GetSourceRange(expr_id)); - auto expr = macro->Expand(factory_, std::ref(target), absl::MakeSpan(args)); - factory_.EndMacro(); - if (expr) { - if (add_macro_calls_) { - factory_.AddMacroCall(expr->id(), function, std::move(macro_target), - std::move(macro_args)); - } - // We did not end up using `expr_id`. Delete metadata. - factory_.EraseId(expr_id); - return std::move(*expr); + } + factory_.BeginMacro(factory_.GetSourceRange(expr_id)); + auto expr = macro->Expand(factory_, std::ref(target), absl::MakeSpan(args)); + factory_.EndMacro(); + if (expr) { + if (add_macro_calls_) { + factory_.AddMacroCall(expr->id(), function, std::move(macro_target), + std::move(macro_args)); } + // We did not end up using `expr_id`. Delete metadata. + factory_.EraseId(expr_id); + return std::move(*expr); } + return factory_.NewMemberCall(expr_id, function, std::move(target), std::move(args)); } @@ -1677,8 +1706,9 @@ absl::StatusOr ParseImpl( CelParser parser(&tokens); ExprRecursionListener listener(options.max_recursion_depth); ParserVisitor visitor( - source, options.max_recursion_depth, registry, options.add_macro_calls, - options.enable_optional_syntax, options.enable_quoted_identifiers, + source, options.max_recursion_depth, options.expression_node_limit, + registry, options.add_macro_calls, options.enable_optional_syntax, + options.enable_quoted_identifiers, options.enable_variadic_logical_operators); lexer.removeErrorListeners(); diff --git a/parser/parser_test.cc b/parser/parser_test.cc index 9ea6e7832..2f0aae3e3 100644 --- a/parser/parser_test.cc +++ b/parser/parser_test.cc @@ -2140,6 +2140,55 @@ TEST_P(ParserTest, ParseFailurePopulatesIssues) { EXPECT_EQ(issues[0].location().column, 3); } +TEST_P(ParserTest, ExpressionNodeLimitExceeded) { + auto builder = cel::NewParserBuilder(options_); + builder->GetOptions().expression_node_limit = 2; + ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build()); + + ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("a + b + c", "test.cel")); + std::vector issues; + auto ast_result = parser->Parse(*source, &issues); + EXPECT_THAT(ast_result, Not(IsOk())); + ASSERT_THAT(issues, testing::Not(testing::IsEmpty())); + EXPECT_THAT(ast_result.status().message(), + HasSubstr("expression node limit exceeded")); + EXPECT_THAT(issues[0].message(), HasSubstr("expression node limit exceeded")); +} + +TEST_P(ParserTest, MacroExpansionNodeLimitExceeded) { + auto builder = cel::NewParserBuilder(options_); + builder->GetOptions().expression_node_limit = 5; + ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build()); + + ASSERT_OK_AND_ASSIGN( + auto source, cel::NewSource("[1, 2, 3, 4, 5].map(x, x * 2)", "test.cel")); + std::vector issues; + auto ast_result = parser->Parse(*source, &issues); + EXPECT_THAT(ast_result, Not(IsOk())); + ASSERT_THAT(issues, testing::Not(testing::IsEmpty())); + EXPECT_THAT(ast_result.status().message(), + HasSubstr("expression node limit exceeded")); + EXPECT_THAT( + issues, + testing::Contains(testing::Property( + &cel::ParseIssue::message, + HasSubstr( + "could not expand macro: expression node limit exceeded")))); +} + +TEST_P(ParserTest, MacroExpansionNodeLimitNotExceeded) { + auto builder = cel::NewParserBuilder(options_); + builder->GetOptions().expression_node_limit = 100; + ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build()); + + ASSERT_OK_AND_ASSIGN( + auto source, cel::NewSource("[1, 2, 3, 4, 5].map(x, x * 2)", "test.cel")); + std::vector issues; + auto ast_result = parser->Parse(*source, &issues); + ASSERT_THAT(ast_result, IsOk()); + EXPECT_THAT(issues, testing::IsEmpty()); +} + std::string ExpressionTestName( const testing::TestParamInfo>& test_info) { const TestInfo& info = std::get<0>(test_info.param);