diff --git a/common/BUILD b/common/BUILD index a1d4ac3bd..937a27f45 100644 --- a/common/BUILD +++ b/common/BUILD @@ -640,8 +640,8 @@ cc_test( deps = [ ":source", "//internal:testing", + "@com_google_absl//absl/status", "@com_google_absl//absl/strings:cord", - "@com_google_absl//absl/types:optional", ], ) diff --git a/common/source.cc b/common/source.cc index 81b1ada9a..e7e09d919 100644 --- a/common/source.cc +++ b/common/source.cc @@ -307,12 +307,18 @@ struct SourceTextTraits { template absl::StatusOr NewSourceImpl(std::string description, const T& text, - const size_t text_size) { + const size_t text_size, + const size_t max_codepoints) { if (ABSL_PREDICT_FALSE( text_size > static_cast(std::numeric_limits::max()))) { return absl::InvalidArgumentError("expression larger than 2GiB limit"); } + if ((text_size >> 2) > max_codepoints) { + // If byte size is 4 times the codepoint limit, then definitely exceeded. + return absl::InvalidArgumentError(absl::StrCat( + "expression is larger than codepoint limit ", max_codepoints)); + } using Traits = SourceTextTraits; size_t index = 0; typename Traits::iterator_type it = Traits::Begin(text); @@ -324,6 +330,10 @@ absl::StatusOr NewSourceImpl(std::string description, const T& text, std::vector data32; absl::InlinedVector line_offsets; while (index < text_size) { + if (offset >= max_codepoints) { + return absl::InvalidArgumentError(absl::StrCat( + "expression is larger than codepoint limit ", max_codepoints)); + } std::tie(code_point, code_units) = cel::internal::Utf8Decode(it); if (ABSL_PREDICT_FALSE(code_point == cel::internal::kUnicodeReplacementCharacter && @@ -375,6 +385,10 @@ absl::StatusOr NewSourceImpl(std::string description, const T& text, std::move(description), std::move(line_offsets), Traits::ToVector(text)); latin1: while (index < text_size) { + if (offset >= max_codepoints) { + return absl::InvalidArgumentError(absl::StrCat( + "expression is larger than codepoint limit ", max_codepoints)); + } std::tie(code_point, code_units) = internal::Utf8Decode(it); if (ABSL_PREDICT_FALSE(code_point == internal::kUnicodeReplacementCharacter && @@ -420,6 +434,10 @@ absl::StatusOr NewSourceImpl(std::string description, const T& text, std::move(description), std::move(line_offsets), std::move(data8)); basic: while (index < text_size) { + if (offset >= max_codepoints) { + return absl::InvalidArgumentError(absl::StrCat( + "expression is larger than codepoint limit ", max_codepoints)); + } std::tie(code_point, code_units) = internal::Utf8Decode(it); if (ABSL_PREDICT_FALSE(code_point == internal::kUnicodeReplacementCharacter && @@ -453,6 +471,10 @@ absl::StatusOr NewSourceImpl(std::string description, const T& text, std::move(description), std::move(line_offsets), std::move(data16)); supplemental: while (index < text_size) { + if (offset >= max_codepoints) { + return absl::InvalidArgumentError(absl::StrCat( + "expression is larger than codepoint limit ", max_codepoints)); + } std::tie(code_point, code_units) = internal::Utf8Decode(it); if (ABSL_PREDICT_FALSE(code_point == internal::kUnicodeReplacementCharacter && @@ -631,16 +653,27 @@ absl::Span SourceSubrange::line_offsets() const { return absl::MakeConstSpan(line_offsets_); } +static size_t ClampLimit(int value) { + if (value < 0) { + return std::numeric_limits::max(); + } + return static_cast(value); +} + absl::StatusOr NewSource(absl::string_view content, - std::string description) { + std::string description, + const SourceOptions& options) { return common_internal::NewSourceImpl(std::move(description), content, - content.size()); + content.size(), + ClampLimit(options.max_codepoint_size)); } absl::StatusOr NewSource(const absl::Cord& content, - std::string description) { + std::string description, + const SourceOptions& options) { return common_internal::NewSourceImpl(std::move(description), content, - content.size()); + content.size(), + ClampLimit(options.max_codepoint_size)); } } // namespace cel diff --git a/common/source.h b/common/source.h index b3968453f..190c750cc 100644 --- a/common/source.h +++ b/common/source.h @@ -221,11 +221,40 @@ class SourceSubrange final : public Source { using SourcePtr = std::unique_ptr; -absl::StatusOr NewSource( - absl::string_view content, std::string description = ""); +struct SourceOptions { + // The maximum number of code points allowed in the source. + // A negative value indicates no limit (though still limited by + // int32_t max value). + int max_codepoint_size = 100'000; +}; + +absl::StatusOr NewSource(absl::string_view content, + std::string description, + const SourceOptions& options); + +absl::StatusOr NewSource(const absl::Cord& content, + std::string description, + const SourceOptions& options); + +inline absl::StatusOr NewSource( + absl::string_view content, std::string description) { + return NewSource(content, std::move(description), SourceOptions{}); +} -absl::StatusOr NewSource( - const absl::Cord& content, std::string description = ""); +inline absl::StatusOr NewSource( + const absl::Cord& content, std::string description) { + return NewSource(content, std::move(description), SourceOptions{}); +} + +inline absl::StatusOr NewSource( + absl::string_view content) { + return NewSource(content, "", SourceOptions{}); +} + +inline absl::StatusOr NewSource( + const absl::Cord& content) { + return NewSource(content, "", SourceOptions{}); +} } // namespace cel diff --git a/common/source_test.cc b/common/source_test.cc index e620d3dd8..8315fd517 100644 --- a/common/source_test.cc +++ b/common/source_test.cc @@ -14,8 +14,11 @@ #include "common/source.h" +#include +#include + +#include "absl/status/status.h" #include "absl/strings/cord.h" -#include "absl/types/optional.h" #include "internal/testing.h" namespace cel { @@ -262,5 +265,51 @@ TEST(SourceSubrange, LineOffsetsMiddleSubrange) { EXPECT_THAT(subrange.line_offsets(), ElementsAre(6, 10)); } +TEST(StringSource, CodepointLimitExceeded) { + SourceOptions options; + options.max_codepoint_size = 5; + + EXPECT_THAT( + NewSource("123456", "test", options), + ::absl_testing::StatusIs( + absl::StatusCode::kInvalidArgument, + ::testing::HasSubstr("expression is larger than codepoint limit 5"))); + + ASSERT_OK_AND_ASSIGN(auto source, NewSource("12345", "test", options)); + EXPECT_THAT(source->content().ToString(), ::testing::Eq("12345")); +} + +TEST(StringSource, CodepointLimitMultibyteUtf8) { + SourceOptions options; + options.max_codepoint_size = 5; + + // "Hello" consists of 5 full-width Unicode characters (15 bytes in + // UTF-8). + ASSERT_OK_AND_ASSIGN(auto source, NewSource("Hello", "test", options)); + EXPECT_THAT(source->content().ToString(), ::testing::Eq("Hello")); + + options.max_codepoint_size = 4; + EXPECT_THAT( + NewSource("Hello", "test", options), + ::absl_testing::StatusIs( + absl::StatusCode::kInvalidArgument, + ::testing::HasSubstr("expression is larger than codepoint limit 4"))); +} + +TEST(CordSource, CodepointLimitExceeded) { + SourceOptions options; + options.max_codepoint_size = 5; + + EXPECT_THAT( + NewSource(absl::Cord("123456"), "test", options), + ::absl_testing::StatusIs( + absl::StatusCode::kInvalidArgument, + ::testing::HasSubstr("expression is larger than codepoint limit 5"))); + + ASSERT_OK_AND_ASSIGN(auto source, + NewSource(absl::Cord("12345"), "test", options)); + EXPECT_THAT(source->content().ToString(), ::testing::Eq("12345")); +} + } // namespace } // namespace cel diff --git a/compiler/compiler.h b/compiler/compiler.h index 174912f71..58c60027b 100644 --- a/compiler/compiler.h +++ b/compiler/compiler.h @@ -178,8 +178,8 @@ class Compiler { inline absl::StatusOr Compiler::Compile( absl::string_view source, absl::string_view description, google::protobuf::Arena* absl_nullable arena) const { - absl::StatusOr source_obj = - NewSource(source, std::string(description)); + absl::StatusOr> source_obj = + GetParser().PrepareSource(source, description); if (!source_obj.ok()) { return source_obj.status(); } diff --git a/compiler/compiler_factory_test.cc b/compiler/compiler_factory_test.cc index 3f83f5bf2..f90cc4862 100644 --- a/compiler/compiler_factory_test.cc +++ b/compiler/compiler_factory_test.cc @@ -441,5 +441,24 @@ TEST(CompilerFactoryTest, CompileSourceOverload) { EXPECT_TRUE(result.IsValid()); } +TEST(CompilerFactoryTest, CodepointLimitExceeded) { + CompilerOptions options; + options.parser_options.expression_size_codepoint_limit = 10; + ASSERT_OK_AND_ASSIGN( + auto builder, + NewCompilerBuilder(cel::internal::GetSharedTestingDescriptorPool(), + options)); + ASSERT_OK_AND_ASSIGN(auto compiler, builder->Build()); + + EXPECT_THAT( + compiler->Compile("123456789012345", "test.cel"), + StatusIs(absl::StatusCode::kInvalidArgument, + HasSubstr("expression is larger than codepoint limit 10"))); + + ASSERT_OK_AND_ASSIGN(ValidationResult result, + compiler->Compile("1234567890", "test.cel")); + EXPECT_TRUE(result.IsValid()); +} + } // namespace } // namespace cel diff --git a/parser/internal/pratt_parser.cc b/parser/internal/pratt_parser.cc index 0ccc28c15..3f59e5065 100644 --- a/parser/internal/pratt_parser.cc +++ b/parser/internal/pratt_parser.cc @@ -163,6 +163,14 @@ absl::StatusOr> PrattParserImpl::ParseImpl( return PrattParseImpl(source, macro_registry_, options_, parse_issues); } +absl::StatusOr> PrattParserImpl::PrepareSourceImpl( + absl::string_view input, absl::string_view description) const { + return cel::NewSource( + input, std::string(description), + cel::SourceOptions{.max_codepoint_size = + options_.expression_size_codepoint_limit}); +} + absl::StatusOr> PrattParseImpl( const cel::Source& source, const cel::MacroRegistry& registry, const ParserOptions& options, std::vector* parse_issues) { diff --git a/parser/internal/pratt_parser.h b/parser/internal/pratt_parser.h index 469f48aec..006a10c28 100644 --- a/parser/internal/pratt_parser.h +++ b/parser/internal/pratt_parser.h @@ -59,6 +59,9 @@ class PrattParserImpl final : public cel::Parser { const cel::Source& source, std::vector* absl_nullable parse_issues) const override; + absl::StatusOr> PrepareSourceImpl( + absl::string_view input, absl::string_view description) const override; + std::unique_ptr ToBuilder() const override; private: diff --git a/parser/parser.cc b/parser/parser.cc index 9bc471fdf..28923e02d 100644 --- a/parser/parser.cc +++ b/parser/parser.cc @@ -1781,6 +1781,14 @@ class ParserImpl : public cel::Parser { std::move(parse_result.source_info)); } + absl::StatusOr> PrepareSourceImpl( + absl::string_view input, absl::string_view description) const override { + return cel::NewSource( + input, std::string(description), + cel::SourceOptions{.max_codepoint_size = + options_.expression_size_codepoint_limit}); + } + std::unique_ptr ToBuilder() const override; private: diff --git a/parser/parser_interface.h b/parser/parser_interface.h index ad6e8ca84..eb902c041 100644 --- a/parser/parser_interface.h +++ b/parser/parser_interface.h @@ -114,6 +114,14 @@ class Parser { absl::StatusOr> Parse( const cel::Source& source, std::vector* issues) const; + // Returns a Source object from the given input. + // Validates that the input is well-formed utf-8 and within the configured + // source limits. + absl::StatusOr> PrepareSource( + absl::string_view input, absl::string_view description) const; + absl::StatusOr> PrepareSource( + absl::string_view input) const; + // Returns a builder initialized with the configuration of this parser. virtual std::unique_ptr ToBuilder() const = 0; @@ -121,6 +129,9 @@ class Parser { virtual absl::StatusOr> ParseImpl( const cel::Source& source, std::vector* absl_nullable parse_issues) const = 0; + + virtual absl::StatusOr> PrepareSourceImpl( + absl::string_view input, absl::string_view description) const = 0; }; inline absl::StatusOr> Parser::Parse( @@ -134,6 +145,16 @@ inline absl::StatusOr> Parser::Parse( return ParseImpl(source, issues); } +inline absl::StatusOr> Parser::PrepareSource( + absl::string_view input, absl::string_view description) const { + return PrepareSourceImpl(input, description); +} + +inline absl::StatusOr> Parser::PrepareSource( + absl::string_view input) const { + return PrepareSourceImpl(input, ""); +} + } // namespace cel #endif // THIRD_PARTY_CEL_CPP_PARSER_PARSER_INTERFACE_H_ diff --git a/parser/parser_test.cc b/parser/parser_test.cc index c2fd36112..c04d819b2 100644 --- a/parser/parser_test.cc +++ b/parser/parser_test.cc @@ -2189,6 +2189,26 @@ TEST_P(ParserTest, MacroExpansionNodeLimitNotExceeded) { EXPECT_THAT(issues, testing::IsEmpty()); } +TEST_P(ParserTest, PrepareSourceForwardsCodepointLimit) { + auto builder = cel::NewParserBuilder(options_); + builder->GetOptions().expression_size_codepoint_limit = 10; + ASSERT_OK_AND_ASSIGN(auto parser, std::move(*builder).Build()); + + EXPECT_THAT(parser->PrepareSource("123456789012345", "test.cel"), + absl_testing::StatusIs( + absl::StatusCode::kInvalidArgument, + HasSubstr("expression is larger than codepoint limit 10"))); + + ASSERT_OK_AND_ASSIGN(auto source, + parser->PrepareSource("1234567890", "test.cel")); + EXPECT_EQ(source->description(), "test.cel"); + EXPECT_EQ(source->content().ToString(), "1234567890"); + + ASSERT_OK_AND_ASSIGN(auto source_default_desc, + parser->PrepareSource("1234567890")); + EXPECT_EQ(source_default_desc->description(), ""); +} + std::string ExpressionTestName( const testing::TestParamInfo>& test_info) { const TestInfo& info = std::get<0>(test_info.param);