Skip to content

Commit ef155f8

Browse files
jnthntatumcopybara-github
authored andcommitted
Add option to set input expression size limit in type checker.
If exceeded, type checking fails early instead of fully visiting the input AST. PiperOrigin-RevId: 698088256
1 parent 19fcb02 commit ef155f8

File tree

3 files changed

+42
-3
lines changed

3 files changed

+42
-3
lines changed

checker/checker_options.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ struct CheckerOptions {
4242
// Enabled by default, but can be disabled to preserve the original type name
4343
// as parsed.
4444
bool update_struct_type_names = true;
45+
46+
// Maximum number (inclusive) of expression nodes to check for an input
47+
// expression.
48+
//
49+
// If exceeded, the checker should return a status with code InvalidArgument.
50+
int max_expression_node_count = 100000;
4551
};
4652

4753
} // namespace cel

checker/internal/type_checker_impl.cc

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -458,9 +458,9 @@ class ResolveVisitor : public AstVisitorBase {
458458
// These are handled separately to disambiguate between namespaces and field
459459
// accesses
460460
absl::flat_hash_set<const Expr*> deferred_select_operations_;
461-
absl::Status status_;
462461
std::vector<std::unique_ptr<VariableScope>> comprehension_vars_;
463462
std::vector<ComprehensionScope> comprehension_scopes_;
463+
absl::Status status_;
464464

465465
// References that were resolved and may require AST rewrites.
466466
absl::flat_hash_map<const Expr*, FunctionResolution> functions_;
@@ -1252,8 +1252,23 @@ absl::StatusOr<ValidationResult> TypeCheckerImpl::Check(
12521252

12531253
TraversalOptions opts;
12541254
opts.use_comprehension_callbacks = true;
1255-
AstTraverse(ast_impl.root_expr(), visitor, opts);
1256-
CEL_RETURN_IF_ERROR(visitor.status());
1255+
1256+
auto traversal = AstTraversal::Create(ast_impl.root_expr(), opts);
1257+
for (int step = 0; step < options_.max_expression_node_count * 2; ++step) {
1258+
bool has_next = traversal.Step(visitor);
1259+
if (!visitor.status().ok()) {
1260+
return visitor.status();
1261+
}
1262+
if (!has_next) {
1263+
break;
1264+
}
1265+
}
1266+
1267+
if (!traversal.IsDone()) {
1268+
return absl::InvalidArgumentError(
1269+
absl::StrCat("Max expression node count exceeded: ",
1270+
options_.max_expression_node_count));
1271+
}
12571272

12581273
if (env_.expected_type().has_value()) {
12591274
visitor.AssertExpectedType(ast_impl.root_expr(), *env_.expected_type());

checker/internal/type_checker_impl_test.cc

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ namespace checker_internal {
5757
namespace {
5858

5959
using ::absl_testing::IsOk;
60+
using ::absl_testing::StatusIs;
6061
using ::cel::ast_internal::AstImpl;
6162
using ::cel::ast_internal::Reference;
6263
using ::cel::expr::conformance::proto3::TestAllTypes;
@@ -65,6 +66,7 @@ using ::testing::_;
6566
using ::testing::Contains;
6667
using ::testing::ElementsAre;
6768
using ::testing::Eq;
69+
using ::testing::HasSubstr;
6870
using ::testing::IsEmpty;
6971
using ::testing::Pair;
7072
using ::testing::Property;
@@ -1013,6 +1015,22 @@ TEST(TypeCheckerImplTest, NullLiteral) {
10131015
EXPECT_TRUE(ast_impl.type_map()[1].has_null());
10141016
}
10151017

1018+
TEST(TypeCheckerImplTest, ExpressionLimitInclusive) {
1019+
TypeCheckEnv env(GetSharedTestingDescriptorPool());
1020+
CheckerOptions options;
1021+
options.max_expression_node_count = 2;
1022+
TypeCheckerImpl impl(std::move(env), options);
1023+
ASSERT_OK_AND_ASSIGN(auto ast, MakeTestParsedAst("{}.foo"));
1024+
ASSERT_OK_AND_ASSIGN(ValidationResult result, impl.Check(std::move(ast)));
1025+
1026+
ASSERT_TRUE(result.IsValid());
1027+
1028+
ASSERT_OK_AND_ASSIGN(ast, MakeTestParsedAst("{}.foo.bar"));
1029+
EXPECT_THAT(impl.Check(std::move(ast)),
1030+
StatusIs(absl::StatusCode::kInvalidArgument,
1031+
HasSubstr("expression node count exceeded: 2")));
1032+
}
1033+
10161034
TEST(TypeCheckerImplTest, ComprehensionUnsupportedRange) {
10171035
TypeCheckEnv env(GetSharedTestingDescriptorPool());
10181036
google::protobuf::Arena arena;

0 commit comments

Comments
 (0)