Skip to content

Commit 4eee1ed

Browse files
jnthntatumcopybara-github
jnthntatum
authored and
copybara-github
committed
Check for unexpected target (function call receiver) expressions in builtin operator handlers.
PiperOrigin-RevId: 744027012
1 parent 00349ca commit 4eee1ed

2 files changed

Lines changed: 144 additions & 3 deletions

File tree

‎eval/compiler/flat_expr_builder.cc‎

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1962,6 +1962,11 @@ FlatExprVisitor::CallHandlerResult FlatExprVisitor::HandleIndex(
19621962
const cel::Expr& expr, const cel::CallExpr& call_expr) {
19631963
ABSL_DCHECK(call_expr.function() == cel::builtin::kIndex);
19641964
auto depth = RecursionEligible();
1965+
if (!ValidateOrError(
1966+
call_expr.args().size() == 2 && !call_expr.has_target(),
1967+
"unexpected number of args for builtin index operator")) {
1968+
return CallHandlerResult::kIntercepted;
1969+
}
19651970

19661971
if (depth.has_value()) {
19671972
auto args = ExtractRecursiveDependencies();
@@ -1984,6 +1989,12 @@ FlatExprVisitor::CallHandlerResult FlatExprVisitor::HandleIndex(
19841989
FlatExprVisitor::CallHandlerResult FlatExprVisitor::HandleNot(
19851990
const cel::Expr& expr, const cel::CallExpr& call_expr) {
19861991
ABSL_DCHECK(call_expr.function() == cel::builtin::kNot);
1992+
1993+
if (!ValidateOrError(call_expr.args().size() == 1 && !call_expr.has_target(),
1994+
"unexpected number of args for builtin not operator")) {
1995+
return CallHandlerResult::kIntercepted;
1996+
}
1997+
19871998
auto depth = RecursionEligible();
19881999

19892000
if (depth.has_value()) {
@@ -2005,6 +2016,12 @@ FlatExprVisitor::CallHandlerResult FlatExprVisitor::HandleNotStrictlyFalse(
20052016
const cel::Expr& expr, const cel::CallExpr& call_expr) {
20062017
auto depth = RecursionEligible();
20072018

2019+
if (!ValidateOrError(call_expr.args().size() == 1 && !call_expr.has_target(),
2020+
"unexpected number of args for builtin "
2021+
"not_strictly_false operator")) {
2022+
return CallHandlerResult::kIntercepted;
2023+
}
2024+
20082025
if (depth.has_value()) {
20092026
auto args = ExtractRecursiveDependencies();
20102027
if (args.size() != 1) {
@@ -2026,7 +2043,7 @@ FlatExprVisitor::CallHandlerResult FlatExprVisitor::HandleBlock(
20262043
const cel::Expr& expr, const cel::CallExpr& call_expr) {
20272044
ABSL_DCHECK(call_expr.function() == kBlock);
20282045
if (!block_.has_value() || block_->expr != &expr ||
2029-
call_expr.args().size() != 2) {
2046+
call_expr.args().size() != 2 || call_expr.has_target()) {
20302047
SetProgressStatusError(
20312048
absl::InvalidArgumentError("unexpected call to internal cel.@block"));
20322049
return CallHandlerResult::kIntercepted;
@@ -2102,7 +2119,7 @@ FlatExprVisitor::CallHandlerResult FlatExprVisitor::HandleListAppend(
21022119
FlatExprVisitor::CallHandlerResult FlatExprVisitor::HandleHeterogeneousEquality(
21032120
const cel::Expr& expr, const cel::CallExpr& call, bool inequality) {
21042121
if (!ValidateOrError(
2105-
call.args().size() == 2,
2122+
call.args().size() == 2 && !call.has_target(),
21062123
"unexpected number of args for builtin equality operator")) {
21072124
return CallHandlerResult::kIntercepted;
21082125
}
@@ -2128,7 +2145,7 @@ FlatExprVisitor::CallHandlerResult FlatExprVisitor::HandleHeterogeneousEquality(
21282145
FlatExprVisitor::CallHandlerResult
21292146
FlatExprVisitor::HandleHeterogeneousEqualityIn(const cel::Expr& expr,
21302147
const cel::CallExpr& call) {
2131-
if (!ValidateOrError(call.args().size() == 2,
2148+
if (!ValidateOrError(call.args().size() == 2 && !call.has_target(),
21322149
"unexpected number of args for builtin 'in' operator")) {
21332150
return CallHandlerResult::kIntercepted;
21342151
}

‎eval/compiler/flat_expr_builder_test.cc‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1916,6 +1916,130 @@ TEST(FlatExprBuilderTest, FastEquality) {
19161916
EXPECT_FALSE(result.BoolOrDie());
19171917
}
19181918

1919+
TEST(FlatExprBuilderTest, FastEqualityFiltersBadCalls) {
1920+
ASSERT_OK_AND_ASSIGN(ParsedExpr parsed_expr, parser::Parse("'foo' == 'bar'"));
1921+
parsed_expr.mutable_expr()
1922+
->mutable_call_expr()
1923+
->mutable_target()
1924+
->mutable_const_expr()
1925+
->set_string_value("foo");
1926+
cel::RuntimeOptions options;
1927+
options.enable_fast_builtins = true;
1928+
InterpreterOptions legacy_options;
1929+
legacy_options.enable_fast_builtins = true;
1930+
CelExpressionBuilderFlatImpl builder(NewTestingRuntimeEnv(), options);
1931+
ASSERT_THAT(RegisterBuiltinFunctions(builder.GetRegistry(), legacy_options),
1932+
IsOk());
1933+
ASSERT_THAT(
1934+
builder.CreateExpression(&parsed_expr.expr(), &parsed_expr.source_info()),
1935+
StatusIs(absl::StatusCode::kInvalidArgument,
1936+
HasSubstr(
1937+
"unexpected number of args for builtin equality operator")));
1938+
}
1939+
1940+
TEST(FlatExprBuilderTest, FastInequalityFiltersBadCalls) {
1941+
ASSERT_OK_AND_ASSIGN(ParsedExpr parsed_expr, parser::Parse("'foo' != 'bar'"));
1942+
parsed_expr.mutable_expr()
1943+
->mutable_call_expr()
1944+
->mutable_target()
1945+
->mutable_const_expr()
1946+
->set_string_value("foo");
1947+
cel::RuntimeOptions options;
1948+
options.enable_fast_builtins = true;
1949+
InterpreterOptions legacy_options;
1950+
legacy_options.enable_fast_builtins = true;
1951+
CelExpressionBuilderFlatImpl builder(NewTestingRuntimeEnv(), options);
1952+
ASSERT_THAT(RegisterBuiltinFunctions(builder.GetRegistry(), legacy_options),
1953+
IsOk());
1954+
ASSERT_THAT(
1955+
builder.CreateExpression(&parsed_expr.expr(), &parsed_expr.source_info()),
1956+
StatusIs(absl::StatusCode::kInvalidArgument,
1957+
HasSubstr(
1958+
"unexpected number of args for builtin equality operator")));
1959+
}
1960+
1961+
TEST(FlatExprBuilderTest, FastInFiltersBadCalls) {
1962+
ASSERT_OK_AND_ASSIGN(ParsedExpr parsed_expr, parser::Parse("a in b"));
1963+
parsed_expr.mutable_expr()
1964+
->mutable_call_expr()
1965+
->mutable_target()
1966+
->mutable_const_expr()
1967+
->set_string_value("foo");
1968+
cel::RuntimeOptions options;
1969+
options.enable_fast_builtins = true;
1970+
InterpreterOptions legacy_options;
1971+
legacy_options.enable_fast_builtins = true;
1972+
CelExpressionBuilderFlatImpl builder(NewTestingRuntimeEnv(), options);
1973+
ASSERT_THAT(RegisterBuiltinFunctions(builder.GetRegistry(), legacy_options),
1974+
IsOk());
1975+
ASSERT_THAT(
1976+
builder.CreateExpression(&parsed_expr.expr(), &parsed_expr.source_info()),
1977+
StatusIs(
1978+
absl::StatusCode::kInvalidArgument,
1979+
HasSubstr("unexpected number of args for builtin 'in' operator")));
1980+
}
1981+
1982+
TEST(FlatExprBuilderTest, IndexFiltersBadCalls) {
1983+
ASSERT_OK_AND_ASSIGN(ParsedExpr parsed_expr, parser::Parse("a[b]"));
1984+
parsed_expr.mutable_expr()
1985+
->mutable_call_expr()
1986+
->mutable_target()
1987+
->mutable_const_expr()
1988+
->set_string_value("foo");
1989+
cel::RuntimeOptions options;
1990+
options.enable_fast_builtins = true;
1991+
InterpreterOptions legacy_options;
1992+
legacy_options.enable_fast_builtins = true;
1993+
CelExpressionBuilderFlatImpl builder(NewTestingRuntimeEnv(), options);
1994+
ASSERT_THAT(RegisterBuiltinFunctions(builder.GetRegistry(), legacy_options),
1995+
IsOk());
1996+
ASSERT_THAT(
1997+
builder.CreateExpression(&parsed_expr.expr(), &parsed_expr.source_info()),
1998+
StatusIs(
1999+
absl::StatusCode::kInvalidArgument,
2000+
HasSubstr("unexpected number of args for builtin index operator")));
2001+
}
2002+
2003+
TEST(FlatExprBuilderTest, NotFiltersBadCalls) {
2004+
ASSERT_OK_AND_ASSIGN(ParsedExpr parsed_expr, parser::Parse("!a"));
2005+
parsed_expr.mutable_expr()
2006+
->mutable_call_expr()
2007+
->mutable_target()
2008+
->mutable_const_expr()
2009+
->set_string_value("foo");
2010+
cel::RuntimeOptions options;
2011+
options.enable_fast_builtins = true;
2012+
InterpreterOptions legacy_options;
2013+
legacy_options.enable_fast_builtins = true;
2014+
CelExpressionBuilderFlatImpl builder(NewTestingRuntimeEnv(), options);
2015+
ASSERT_THAT(RegisterBuiltinFunctions(builder.GetRegistry(), legacy_options),
2016+
IsOk());
2017+
ASSERT_THAT(
2018+
builder.CreateExpression(&parsed_expr.expr(), &parsed_expr.source_info()),
2019+
StatusIs(
2020+
absl::StatusCode::kInvalidArgument,
2021+
HasSubstr("unexpected number of args for builtin not operator")));
2022+
}
2023+
2024+
TEST(FlatExprBuilderTest, NotStrictlyFalseFiltersBadCalls) {
2025+
ASSERT_OK_AND_ASSIGN(ParsedExpr parsed_expr, parser::Parse("!a"));
2026+
auto* call = parsed_expr.mutable_expr()->mutable_call_expr();
2027+
call->mutable_target()->mutable_const_expr()->set_string_value("foo");
2028+
call->set_function("@not_strictly_false");
2029+
cel::RuntimeOptions options;
2030+
options.enable_fast_builtins = true;
2031+
InterpreterOptions legacy_options;
2032+
legacy_options.enable_fast_builtins = true;
2033+
CelExpressionBuilderFlatImpl builder(NewTestingRuntimeEnv(), options);
2034+
ASSERT_THAT(RegisterBuiltinFunctions(builder.GetRegistry(), legacy_options),
2035+
IsOk());
2036+
ASSERT_THAT(
2037+
builder.CreateExpression(&parsed_expr.expr(), &parsed_expr.source_info()),
2038+
StatusIs(absl::StatusCode::kInvalidArgument,
2039+
HasSubstr("unexpected number of args for builtin "
2040+
"not_strictly_false operator")));
2041+
}
2042+
19192043
TEST(FlatExprBuilderTest, FastEqualityDisabledWithCustomEquality) {
19202044
TestMessage message;
19212045
ASSERT_OK_AND_ASSIGN(ParsedExpr parsed_expr, parser::Parse("1 == b'\001'"));

0 commit comments

Comments
 (0)