Skip to content

Commit 02b617c

Browse files
jnthntatumcopybara-github
jnthntatum
authored and
copybara-github
committed
Add limit for expression nodes in parsed ASTs.
This is mainly to prevent misbehaving macros from generating an AST that is much larger than expected (not directly proportional to source length). PiperOrigin-RevId: 954860770
1 parent db06cd9 commit 02b617c

6 files changed

Lines changed: 151 additions & 47 deletions

File tree

‎parser/internal/options.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ inline constexpr int kDefaultErrorRecoveryLimit = 12;
2121
inline constexpr int kDefaultMaxRecursionDepth = 32;
2222
inline constexpr int kExpressionSizeCodepointLimit = 100'000;
2323
inline constexpr int kDefaultErrorRecoveryTokenLookaheadLimit = 512;
24+
inline constexpr int kDefaultExpressionNodeLimit = 100'000;
2425
inline constexpr bool kDefaultAddMacroCalls = false;
2526

2627
} // namespace cel::parser_internal

‎parser/internal/pratt_parser_worker.cc‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,14 +192,24 @@ void ParserWorker::SynchronizeOnDelimiter() {
192192
NextToken();
193193
}
194194
}
195+
195196
int64_t ParserWorker::NextId(int32_t position) {
196197
int64_t id = next_id_++;
198+
if (id > options_.expression_node_limit) {
199+
ReportError(position, "expression node limit exceeded");
200+
}
197201
if (position >= 0) {
198202
positions_.insert({id, position});
199203
}
200204
return id;
201205
}
202206

207+
int64_t ParserWorker::NextId() { return NextId(-1); }
208+
209+
bool ParserWorker::NodeLimitExceeded() {
210+
return next_id_ > options_.expression_node_limit;
211+
}
212+
203213
int64_t ParserWorker::CopyId(int64_t id) {
204214
if (id == 0) {
205215
return 0;

‎parser/internal/pratt_parser_worker.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ class ParserWorker {
7676
// ID and Position tracking
7777
int64_t NextId(int32_t position);
7878
int64_t NextId(const Token& token) { return NextId(token.start); }
79-
int64_t NextId() { return next_id_++; }
79+
int64_t NextId();
80+
bool NodeLimitExceeded();
8081
int64_t CopyId(int64_t id);
8182
void EraseId(int64_t id);
8283

@@ -1050,6 +1051,11 @@ std::optional<ExprNode> PrattParserWorker<ExprNode>::TryExpandMacro(
10501051
if (!expander) {
10511052
return std::nullopt;
10521053
}
1054+
if (NodeLimitExceeded()) {
1055+
ReportError(expr_id,
1056+
"could not expand macro: expression node limit exceeded");
1057+
return std::nullopt;
1058+
}
10531059

10541060
std::vector<ExprNode> macro_args;
10551061
ExprNode macro_target;

‎parser/options.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,14 @@ struct ParserOptions final {
4242
int error_recovery_token_lookahead_limit =
4343
::cel::parser_internal::kDefaultErrorRecoveryTokenLookaheadLimit;
4444

45+
// Limit on the number of expression nodes in the abstract syntax tree for the
46+
// expression. This prevents cases where macro expansion results in an AST
47+
// that is larger than expected from the source expression. Once exceeded,
48+
// the parser will record an error and stop expanding macros but continue
49+
// parsing to report other errors.
50+
int expression_node_limit =
51+
::cel::parser_internal::kDefaultExpressionNodeLimit;
52+
4553
// Add macro calls to macro_calls list in source_info.
4654
bool add_macro_calls = ::cel::parser_internal::kDefaultAddMacroCalls;
4755

‎parser/parser.cc‎

Lines changed: 76 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,9 @@ SourceRange SourceRangeFromParserRuleContext(
165165

166166
class ParserMacroExprFactory final : public MacroExprFactory {
167167
public:
168-
explicit ParserMacroExprFactory(const cel::Source& source)
169-
: source_(source) {}
168+
explicit ParserMacroExprFactory(const cel::Source& source,
169+
int expression_node_limit)
170+
: source_(source), expression_node_limit_(expression_node_limit) {}
170171

171172
void BeginMacro(SourceRange macro_position) {
172173
macro_position_ = macro_position;
@@ -203,12 +204,18 @@ class ParserMacroExprFactory final : public MacroExprFactory {
203204

204205
int64_t NextId(const SourceRange& range) {
205206
auto id = expr_id_++;
207+
if (id > expression_node_limit_ && !node_limit_exceeded_) {
208+
node_limit_exceeded_ = true;
209+
ReportError(range, "expression node limit exceeded");
210+
}
206211
if (range.begin != -1 || range.end != -1) {
207212
positions_.insert(std::pair{id, range});
208213
}
209214
return id;
210215
}
211216

217+
bool is_node_limit_exceeded() const { return node_limit_exceeded_; }
218+
212219
bool HasErrors() const { return error_count_ != 0; }
213220

214221
std::vector<cel::ParseIssue> CollectIssues() {
@@ -409,6 +416,8 @@ class ParserMacroExprFactory final : public MacroExprFactory {
409416
std::vector<ParserError> errors_;
410417
size_t error_count_ = 0;
411418
const Source& source_;
419+
int expression_node_limit_;
420+
bool node_limit_exceeded_ = false;
412421
SourceRange macro_position_;
413422
};
414423

@@ -623,13 +632,14 @@ class ParserVisitor final : public CelBaseVisitor,
623632
public antlr4::BaseErrorListener {
624633
public:
625634
ParserVisitor(const cel::Source& source, int max_recursion_depth,
635+
int max_expression_node_count,
626636
const cel::MacroRegistry& macro_registry,
627637
bool add_macro_calls = false,
628638
bool enable_optional_syntax = false,
629639
bool enable_quoted_identifiers = false,
630640
bool enable_variadic_logical_operators = false)
631641
: source_(source),
632-
factory_(source_),
642+
factory_(source_, max_expression_node_count),
633643
macro_registry_(macro_registry),
634644
recursion_depth_(0),
635645
max_recursion_depth_(max_recursion_depth),
@@ -1227,6 +1237,8 @@ std::vector<ListExprElement> ParserVisitor::visitList(
12271237
if (!enable_optional_syntax_ && expr_ctx->opt != nullptr) {
12281238
factory_.ReportError(SourceRangeFromParserRuleContext(ctx),
12291239
"unsupported syntax '?'");
1240+
// Still generate an ID to detect node limit exceeded.
1241+
factory_.NextId(SourceRangeFromParserRuleContext(ctx));
12301242
rv.push_back(factory_.NewListElement(factory_.NewUnspecified(0), false));
12311243
continue;
12321244
}
@@ -1298,6 +1310,9 @@ std::vector<MapExprEntry> ParserVisitor::visitEntries(
12981310
if (!enable_optional_syntax_ && ctx->keys[i]->opt) {
12991311
factory_.ReportError(SourceRangeFromParserRuleContext(ctx),
13001312
"unsupported syntax '?'");
1313+
// Still generate an ID to detect node limit exceeded.
1314+
factory_.NextId(SourceRangeFromParserRuleContext(ctx));
1315+
factory_.NextId(SourceRangeFromParserRuleContext(ctx));
13011316
res.push_back(factory_.NewMapEntry(0, factory_.NewUnspecified(0),
13021317
factory_.NewUnspecified(0), false));
13031318
continue;
@@ -1461,60 +1476,74 @@ std::vector<cel::ParseIssue> ParserVisitor::CollectIssues() {
14611476
Expr ParserVisitor::GlobalCallOrMacroImpl(int64_t expr_id,
14621477
absl::string_view function,
14631478
std::vector<Expr> args) {
1464-
if (auto macro = macro_registry_.FindMacro(function, args.size(), false);
1465-
macro) {
1466-
std::vector<Expr> macro_args;
1467-
if (add_macro_calls_) {
1468-
macro_args.reserve(args.size());
1469-
for (const auto& arg : args) {
1470-
macro_args.push_back(factory_.BuildMacroCallArg(arg));
1471-
}
1479+
auto macro = macro_registry_.FindMacro(function, args.size(), false);
1480+
if (!macro) {
1481+
return factory_.NewCall(expr_id, function, std::move(args));
1482+
}
1483+
if (factory_.is_node_limit_exceeded()) {
1484+
return factory_.ReportError(
1485+
factory_.GetSourceRange(expr_id),
1486+
"could not expand macro: expression node limit exceeded");
1487+
}
1488+
std::vector<Expr> macro_args;
1489+
if (add_macro_calls_) {
1490+
macro_args.reserve(args.size());
1491+
for (const auto& arg : args) {
1492+
macro_args.push_back(factory_.BuildMacroCallArg(arg));
14721493
}
1473-
factory_.BeginMacro(factory_.GetSourceRange(expr_id));
1474-
auto expr = macro->Expand(factory_, std::nullopt, absl::MakeSpan(args));
1475-
factory_.EndMacro();
1476-
if (expr) {
1477-
if (add_macro_calls_) {
1478-
factory_.AddMacroCall(expr->id(), function, std::nullopt,
1479-
std::move(macro_args));
1480-
}
1481-
// We did not end up using `expr_id`. Delete metadata.
1482-
factory_.EraseId(expr_id);
1483-
return std::move(*expr);
1494+
}
1495+
factory_.BeginMacro(factory_.GetSourceRange(expr_id));
1496+
auto expr = macro->Expand(factory_, std::nullopt, absl::MakeSpan(args));
1497+
factory_.EndMacro();
1498+
if (expr) {
1499+
if (add_macro_calls_) {
1500+
factory_.AddMacroCall(expr->id(), function, std::nullopt,
1501+
std::move(macro_args));
14841502
}
1503+
// We did not end up using `expr_id`. Delete metadata.
1504+
factory_.EraseId(expr_id);
1505+
return std::move(*expr);
14851506
}
1486-
14871507
return factory_.NewCall(expr_id, function, std::move(args));
14881508
}
14891509

14901510
Expr ParserVisitor::ReceiverCallOrMacroImpl(int64_t expr_id,
14911511
absl::string_view function,
14921512
Expr target,
14931513
std::vector<Expr> args) {
1494-
if (auto macro = macro_registry_.FindMacro(function, args.size(), true);
1495-
macro) {
1496-
Expr macro_target;
1497-
std::vector<Expr> macro_args;
1498-
if (add_macro_calls_) {
1499-
macro_args.reserve(args.size());
1500-
macro_target = factory_.BuildMacroCallArg(target);
1501-
for (const auto& arg : args) {
1502-
macro_args.push_back(factory_.BuildMacroCallArg(arg));
1503-
}
1514+
auto macro = macro_registry_.FindMacro(function, args.size(), true);
1515+
if (!macro) {
1516+
return factory_.NewMemberCall(expr_id, function, std::move(target),
1517+
std::move(args));
1518+
}
1519+
if (factory_.is_node_limit_exceeded()) {
1520+
return factory_.ReportError(
1521+
factory_.GetSourceRange(expr_id),
1522+
"could not expand macro: expression node limit exceeded");
1523+
}
1524+
1525+
Expr macro_target;
1526+
std::vector<Expr> macro_args;
1527+
if (add_macro_calls_) {
1528+
macro_args.reserve(args.size());
1529+
macro_target = factory_.BuildMacroCallArg(target);
1530+
for (const auto& arg : args) {
1531+
macro_args.push_back(factory_.BuildMacroCallArg(arg));
15041532
}
1505-
factory_.BeginMacro(factory_.GetSourceRange(expr_id));
1506-
auto expr = macro->Expand(factory_, std::ref(target), absl::MakeSpan(args));
1507-
factory_.EndMacro();
1508-
if (expr) {
1509-
if (add_macro_calls_) {
1510-
factory_.AddMacroCall(expr->id(), function, std::move(macro_target),
1511-
std::move(macro_args));
1512-
}
1513-
// We did not end up using `expr_id`. Delete metadata.
1514-
factory_.EraseId(expr_id);
1515-
return std::move(*expr);
1533+
}
1534+
factory_.BeginMacro(factory_.GetSourceRange(expr_id));
1535+
auto expr = macro->Expand(factory_, std::ref(target), absl::MakeSpan(args));
1536+
factory_.EndMacro();
1537+
if (expr) {
1538+
if (add_macro_calls_) {
1539+
factory_.AddMacroCall(expr->id(), function, std::move(macro_target),
1540+
std::move(macro_args));
15161541
}
1542+
// We did not end up using `expr_id`. Delete metadata.
1543+
factory_.EraseId(expr_id);
1544+
return std::move(*expr);
15171545
}
1546+
15181547
return factory_.NewMemberCall(expr_id, function, std::move(target),
15191548
std::move(args));
15201549
}
@@ -1677,8 +1706,9 @@ absl::StatusOr<ParseResult> ParseImpl(
16771706
CelParser parser(&tokens);
16781707
ExprRecursionListener listener(options.max_recursion_depth);
16791708
ParserVisitor visitor(
1680-
source, options.max_recursion_depth, registry, options.add_macro_calls,
1681-
options.enable_optional_syntax, options.enable_quoted_identifiers,
1709+
source, options.max_recursion_depth, options.expression_node_limit,
1710+
registry, options.add_macro_calls, options.enable_optional_syntax,
1711+
options.enable_quoted_identifiers,
16821712
options.enable_variadic_logical_operators);
16831713

16841714
lexer.removeErrorListeners();

‎parser/parser_test.cc‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2140,6 +2140,55 @@ TEST_P(ParserTest, ParseFailurePopulatesIssues) {
21402140
EXPECT_EQ(issues[0].location().column, 3);
21412141
}
21422142

2143+
TEST_P(ParserTest, ExpressionNodeLimitExceeded) {
2144+
auto builder = cel::NewParserBuilder(options_);
2145+
builder->GetOptions().expression_node_limit = 2;
2146+
ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build());
2147+
2148+
ASSERT_OK_AND_ASSIGN(auto source, cel::NewSource("a + b + c", "test.cel"));
2149+
std::vector<cel::ParseIssue> issues;
2150+
auto ast_result = parser->Parse(*source, &issues);
2151+
EXPECT_THAT(ast_result, Not(IsOk()));
2152+
ASSERT_THAT(issues, testing::Not(testing::IsEmpty()));
2153+
EXPECT_THAT(ast_result.status().message(),
2154+
HasSubstr("expression node limit exceeded"));
2155+
EXPECT_THAT(issues[0].message(), HasSubstr("expression node limit exceeded"));
2156+
}
2157+
2158+
TEST_P(ParserTest, MacroExpansionNodeLimitExceeded) {
2159+
auto builder = cel::NewParserBuilder(options_);
2160+
builder->GetOptions().expression_node_limit = 5;
2161+
ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build());
2162+
2163+
ASSERT_OK_AND_ASSIGN(
2164+
auto source, cel::NewSource("[1, 2, 3, 4, 5].map(x, x * 2)", "test.cel"));
2165+
std::vector<cel::ParseIssue> issues;
2166+
auto ast_result = parser->Parse(*source, &issues);
2167+
EXPECT_THAT(ast_result, Not(IsOk()));
2168+
ASSERT_THAT(issues, testing::Not(testing::IsEmpty()));
2169+
EXPECT_THAT(ast_result.status().message(),
2170+
HasSubstr("expression node limit exceeded"));
2171+
EXPECT_THAT(
2172+
issues,
2173+
testing::Contains(testing::Property(
2174+
&cel::ParseIssue::message,
2175+
HasSubstr(
2176+
"could not expand macro: expression node limit exceeded"))));
2177+
}
2178+
2179+
TEST_P(ParserTest, MacroExpansionNodeLimitNotExceeded) {
2180+
auto builder = cel::NewParserBuilder(options_);
2181+
builder->GetOptions().expression_node_limit = 100;
2182+
ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build());
2183+
2184+
ASSERT_OK_AND_ASSIGN(
2185+
auto source, cel::NewSource("[1, 2, 3, 4, 5].map(x, x * 2)", "test.cel"));
2186+
std::vector<cel::ParseIssue> issues;
2187+
auto ast_result = parser->Parse(*source, &issues);
2188+
ASSERT_THAT(ast_result, IsOk());
2189+
EXPECT_THAT(issues, testing::IsEmpty());
2190+
}
2191+
21432192
std::string ExpressionTestName(
21442193
const testing::TestParamInfo<std::tuple<TestInfo, bool>>& test_info) {
21452194
const TestInfo& info = std::get<0>(test_info.param);

0 commit comments

Comments
 (0)