From 8fa6426c2055cfd0f6075fd9e3ee56ffeb3949b6 Mon Sep 17 00:00:00 2001 From: Asra Ali Date: Thu, 24 Oct 2019 10:54:12 -0400 Subject: [PATCH 1/9] fuzz: add cel expression fuzzer Signed-off-by: Asra Ali --- .../extensions/filters/common/expr/context.cc | 10 ++- test/extensions/filters/common/expr/BUILD | 24 +++++++ .../expr/evaluator_corpus/errorcondition | 39 ++++++++++ .../common/expr/evaluator_corpus/example | 26 +++++++ .../common/expr/evaluator_corpus/example1 | 31 ++++++++ .../expr/evaluator_corpus/headercondition | 47 ++++++++++++ .../expr/evaluator_corpus/metadatacondition | 63 ++++++++++++++++ .../expr/evaluator_corpus/mistypedcondition | 26 +++++++ .../filters/common/expr/evaluator_fuzz.proto | 17 +++++ .../common/expr/evaluator_fuzz_test.cc | 72 +++++++++++++++++++ test/fuzz/common.proto | 1 + 11 files changed, 353 insertions(+), 3 deletions(-) create mode 100644 test/extensions/filters/common/expr/evaluator_corpus/errorcondition create mode 100644 test/extensions/filters/common/expr/evaluator_corpus/example create mode 100644 test/extensions/filters/common/expr/evaluator_corpus/example1 create mode 100644 test/extensions/filters/common/expr/evaluator_corpus/headercondition create mode 100644 test/extensions/filters/common/expr/evaluator_corpus/metadatacondition create mode 100644 test/extensions/filters/common/expr/evaluator_corpus/mistypedcondition create mode 100644 test/extensions/filters/common/expr/evaluator_fuzz.proto create mode 100644 test/extensions/filters/common/expr/evaluator_fuzz_test.cc diff --git a/source/extensions/filters/common/expr/context.cc b/source/extensions/filters/common/expr/context.cc index b9f1f699e9045..a0d85da598b1c 100644 --- a/source/extensions/filters/common/expr/context.cc +++ b/source/extensions/filters/common/expr/context.cc @@ -124,7 +124,7 @@ absl::optional ResponseWrapper::operator[](CelValue key) const { return CelValue::CreateInt64(code.value()); } } else if (value == Size) { - return CelValue::CreateInt64(info_.bytesSent()); + return CelValue::CreateUint64(info_.bytesSent()); } else if (value == Headers) { return CelValue::CreateMap(&headers_); } else if (value == Trailers) { @@ -186,9 +186,13 @@ absl::optional PeerWrapper::operator[](CelValue key) const { auto value = key.StringOrDie().value(); if (value == Address) { if (local_) { - return CelValue::CreateString(info_.downstreamLocalAddress()->asStringView()); + if (info_.downstreamLocalAddress() != nullptr) { + return CelValue::CreateString(info_.downstreamLocalAddress()->asStringView()); + } } else { - return CelValue::CreateString(info_.downstreamRemoteAddress()->asStringView()); + if (info_.downstreamLocalAddress() != nullptr) { + return CelValue::CreateString(info_.downstreamRemoteAddress()->asStringView()); + } } } else if (value == Port) { if (local_) { diff --git a/test/extensions/filters/common/expr/BUILD b/test/extensions/filters/common/expr/BUILD index 8ce5555328bf0..c6af64c0a0f17 100644 --- a/test/extensions/filters/common/expr/BUILD +++ b/test/extensions/filters/common/expr/BUILD @@ -2,7 +2,9 @@ licenses(["notice"]) # Apache 2 load( "//bazel:envoy_build_system.bzl", + "envoy_cc_fuzz_test", "envoy_package", + "envoy_proto_library", ) load( "//test/extensions:extensions_build_system.bzl", @@ -23,3 +25,25 @@ envoy_extension_cc_test( "//test/test_common:utility_lib", ], ) + +envoy_proto_library( + name = "evaluator_fuzz_proto", + srcs = ["evaluator_fuzz.proto"], + deps = [ + "//test/fuzz:common_proto", + "@com_google_googleapis//google/api/expr/v1alpha1:syntax_proto", + ], +) + +envoy_cc_fuzz_test( + name = "evaluator_fuzz_test", + srcs = ["evaluator_fuzz_test.cc"], + corpus = ":evaluator_corpus", + deps = [ + ":evaluator_fuzz_proto_cc_proto", + "//source/extensions/filters/common/expr:evaluator_lib", + "//test/common/stream_info:test_util", + "//test/fuzz:utility_lib", + "//test/test_common:utility_lib", + ], +) diff --git a/test/extensions/filters/common/expr/evaluator_corpus/errorcondition b/test/extensions/filters/common/expr/evaluator_corpus/errorcondition new file mode 100644 index 0000000000000..a57bc99208985 --- /dev/null +++ b/test/extensions/filters/common/expr/evaluator_corpus/errorcondition @@ -0,0 +1,39 @@ +expression { + call_expr { + function: "_[_]" + args { + select_expr { + operand { + ident_expr { + name: "request" + } + } + field: "undefined" + } + const_expr { + string_value: "foo" + } + } + } +} +request_headers { + headers{key: ":method" value : "GET"} + headers{key: ":path" value : "/"} + headers{key: ":scheme" value : "http"} + headers{key: ":authority" value : "foo.com"} + headers {} + headers {} + headers {} + headers {} + headers {} +} +response_headers { + headers { + key: ":status" + value : "200" + } +} +trailers { + headers {} +} +stream_info {} \ No newline at end of file diff --git a/test/extensions/filters/common/expr/evaluator_corpus/example b/test/extensions/filters/common/expr/evaluator_corpus/example new file mode 100644 index 0000000000000..18fff18a2a435 --- /dev/null +++ b/test/extensions/filters/common/expr/evaluator_corpus/example @@ -0,0 +1,26 @@ +expression { + const_expr { + bool_value: false + } +} +request_headers { + headers{key: ":method" value : "GET"} + headers{key: ":path" value : "/"} + headers{key: ":scheme" value : "http"} + headers{key: ":authority" value : "foo.com"} + headers {} + headers {} + headers {} + headers {} + headers {} +} +response_headers { + headers { + key: ":status" + value : "200" + } +} +trailers { + headers {} +} +stream_info {} \ No newline at end of file diff --git a/test/extensions/filters/common/expr/evaluator_corpus/example1 b/test/extensions/filters/common/expr/evaluator_corpus/example1 new file mode 100644 index 0000000000000..0d021a92ac507 --- /dev/null +++ b/test/extensions/filters/common/expr/evaluator_corpus/example1 @@ -0,0 +1,31 @@ +expression { + call_expr { + function: "undefined_extent" + args { + const_expr { + bool_value: false + } + } + } +} +request_headers { + headers{key: ":method" value : "GET"} + headers{key: ":path" value : "/"} + headers{key: ":scheme" value : "http"} + headers{key: ":authority" value : "foo.com"} + headers {} + headers {} + headers {} + headers {} + headers {} +} +response_headers { + headers { + key: ":status" + value : "200" + } +} +trailers { + headers {} +} +stream_info {} \ No newline at end of file diff --git a/test/extensions/filters/common/expr/evaluator_corpus/headercondition b/test/extensions/filters/common/expr/evaluator_corpus/headercondition new file mode 100644 index 0000000000000..641e1a14a5c58 --- /dev/null +++ b/test/extensions/filters/common/expr/evaluator_corpus/headercondition @@ -0,0 +1,47 @@ +expression { + call_expr { + function: "_==_" + args { + call_expr { + function: "_[_]" + args { + select_expr { + operand { + ident_expr { + name: "request" + } + } + field: "headers" + } + const_expr { + string_value: "foo" + } + } + } + const_expr { + string_value: "bar" + } + } + } +} +request_headers { + headers{key: ":method" value : "GET"} + headers{key: ":path" value : "/"} + headers{key: ":scheme" value : "http"} + headers{key: ":authority" value : "foo.com"} + headers {key: "foo" value: "bar"} + headers {} + headers {} + headers {} + headers {} +} +response_headers { + headers { + key: ":status" + value : "200" + } +} +trailers { + headers {} +} +stream_info {} \ No newline at end of file diff --git a/test/extensions/filters/common/expr/evaluator_corpus/metadatacondition b/test/extensions/filters/common/expr/evaluator_corpus/metadatacondition new file mode 100644 index 0000000000000..d109f18b1fdf7 --- /dev/null +++ b/test/extensions/filters/common/expr/evaluator_corpus/metadatacondition @@ -0,0 +1,63 @@ +expression { + call_expr { + function: "_==_" + args { + call_expr { + function: "_[_]" + args { + select_expr { + operand { + ident_expr { + name: "metadata" + } + } + field: "filter_metadata" + } + const_expr { + string_value: "other" + } + } + const_expr { + string_value: "label" + } + } + const_expr { + string_value: "prod" + } + } + } +} +request_headers { + headers{key: ":method" value : "GET"} + headers{key: ":path" value : "/"} + headers{key: ":scheme" value : "http"} + headers{key: ":authority" value : "foo.com"} + headers {key: "foo" value: "bar"} + headers {} + headers {} + headers {} + headers {} +} +response_headers { + headers { + key: ":status" + value : "200" + } +} +trailers { + headers {} +} +stream_info { + start_time: 1522796769123 + upstream_metadata { + filter_metadata { + key: "other" + value: { + fields { + key: "label" + value: { string_value: "prod" } + } + } + } + } +} diff --git a/test/extensions/filters/common/expr/evaluator_corpus/mistypedcondition b/test/extensions/filters/common/expr/evaluator_corpus/mistypedcondition new file mode 100644 index 0000000000000..c65359cbf66d7 --- /dev/null +++ b/test/extensions/filters/common/expr/evaluator_corpus/mistypedcondition @@ -0,0 +1,26 @@ +expression { + const_expr { + int64_value: 13 + } +} +request_headers { + headers{key: ":method" value : "GET"} + headers{key: ":path" value : "/"} + headers{key: ":scheme" value : "http"} + headers{key: ":authority" value : "foo.com"} + headers {} + headers {} + headers {} + headers {} + headers {} +} +response_headers { + headers { + key: ":status" + value : "200" + } +} +trailers { + headers {} +} +stream_info {} \ No newline at end of file diff --git a/test/extensions/filters/common/expr/evaluator_fuzz.proto b/test/extensions/filters/common/expr/evaluator_fuzz.proto new file mode 100644 index 0000000000000..5001d0fdae820 --- /dev/null +++ b/test/extensions/filters/common/expr/evaluator_fuzz.proto @@ -0,0 +1,17 @@ +syntax = "proto3"; + +package test.extensions.filters.common.expr; + +import "google/api/expr/v1alpha1/syntax.proto"; +import "test/fuzz/common.proto"; +import "validate/validate.proto"; + +// Structured input for rbac_fuzz_test. + +message EvaluatorTestCase { + google.api.expr.v1alpha1.Expr expression = 1 [(validate.rules).message.required = true]; + test.fuzz.Headers request_headers = 2; + test.fuzz.Headers response_headers = 3; + test.fuzz.Headers trailers = 4; + test.fuzz.StreamInfo stream_info = 5; +} \ No newline at end of file diff --git a/test/extensions/filters/common/expr/evaluator_fuzz_test.cc b/test/extensions/filters/common/expr/evaluator_fuzz_test.cc new file mode 100644 index 0000000000000..cef7ab7695152 --- /dev/null +++ b/test/extensions/filters/common/expr/evaluator_fuzz_test.cc @@ -0,0 +1,72 @@ +#include "common/network/utility.h" + +#include "extensions/filters/common/expr/evaluator.h" + +#include "test/common/stream_info/test_util.h" +#include "test/extensions/filters/common/expr/evaluator_fuzz.pb.validate.h" +#include "test/fuzz/fuzz_runner.h" +#include "test/fuzz/utility.h" +#include "test/test_common/network_utility.h" +#include "test/test_common/utility.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace Extensions { +namespace Filters { +namespace Common { +namespace Expr { +namespace { + +// Set stream_info using information from input. +// Duplicates most of fromStreamInfo, except is more lightweight without the use of mocks. +TestStreamInfo makeStreamInfo(const test::fuzz::StreamInfo& info) { + TestStreamInfo stream_info; + // TODO(asraa): Set upstreamHost() and downstreamSslConnection(). + stream_info.metadata_ = info.dynamic_metadata(); + const auto start_time = + static_cast(std::numeric_limits::max()) < + info.start_time() + ? 0 + : info.start_time() / 1000; + stream_info.start_time_ = SystemTime(std::chrono::microseconds(start_time)); + stream_info.setRequestedServerName(info.requested_server_name()); + stream_info.response_code_ = info.has_response_code() ? info.response_code().value() : 200; + auto address = Network::Utility::resolveUrl("tcp://10.0.0.1:443"); + stream_info.upstream_local_address_ = address; + stream_info.downstream_local_address_ = address; + stream_info.downstream_direct_remote_address_ = address; + stream_info.downstream_remote_address_ = address; + return stream_info; +} + +DEFINE_PROTO_FUZZER(const test::extensions::filters::common::expr::EvaluatorTestCase& input) { + try { + // Validate that the input has an expression. + TestUtility::validate(input); + + // Create the CEL expression. + Protobuf::Arena arena; + Expr::BuilderPtr builder = Expr::createBuilder(&arena); + Expr::ExpressionPtr expr = Expr::createExpression(*builder, input.expression()); + + // Create the headers and stream_info to test against. + TestStreamInfo stream_info = makeStreamInfo(input.stream_info()); + Http::TestHeaderMapImpl request_headers = Fuzz::fromHeaders(input.request_headers()); + Http::TestHeaderMapImpl response_headers = Fuzz::fromHeaders(input.response_headers()); + Http::TestHeaderMapImpl response_trailers = Fuzz::fromHeaders(input.trailers()); + + // Evaluate the CEL expression. + Expr::evaluate(*expr, &arena, stream_info, &request_headers, &response_headers, + &response_trailers); + } catch (const EnvoyException& e) { + ENVOY_LOG_MISC(debug, "EnvoyException: {}", e.what()); + } +} + +} // namespace +} // namespace Expr +} // namespace Common +} // namespace Filters +} // namespace Extensions +} // namespace Envoy diff --git a/test/fuzz/common.proto b/test/fuzz/common.proto index 11cecec91b7c8..073f5b30e3244 100644 --- a/test/fuzz/common.proto +++ b/test/fuzz/common.proto @@ -17,4 +17,5 @@ message StreamInfo { uint64 start_time = 2; google.protobuf.UInt32Value response_code = 3; envoy.api.v2.core.Metadata upstream_metadata = 4; + string requested_server_name = 5; } From c8c6b1c129d7876dfedd14bfaecbc3298fab2961 Mon Sep 17 00:00:00 2001 From: Asra Ali Date: Thu, 24 Oct 2019 10:55:00 -0400 Subject: [PATCH 2/9] add spelling Signed-off-by: Asra Ali --- tools/spelling_dictionary.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt index dc0ac90adb04c..06e1eb1a4431f 100644 --- a/tools/spelling_dictionary.txt +++ b/tools/spelling_dictionary.txt @@ -19,6 +19,7 @@ BSON CAS CB CDS +CEL CHACHA CHLO CHLOS From 886c859115c375742f6a87b1d9012fe41a25baca Mon Sep 17 00:00:00 2001 From: Asra Ali Date: Thu, 24 Oct 2019 12:21:07 -0400 Subject: [PATCH 3/9] fixup Signed-off-by: Asra Ali --- test/extensions/filters/common/expr/context_test.cc | 4 ++-- test/extensions/filters/common/expr/evaluator_fuzz.proto | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/extensions/filters/common/expr/context_test.cc b/test/extensions/filters/common/expr/context_test.cc index a3b52cc89d1b9..d14172556f1f6 100644 --- a/test/extensions/filters/common/expr/context_test.cc +++ b/test/extensions/filters/common/expr/context_test.cc @@ -209,8 +209,8 @@ TEST(Context, ResponseAttributes) { { auto value = response[CelValue::CreateString(Size)]; EXPECT_TRUE(value.has_value()); - ASSERT_TRUE(value.value().IsInt64()); - EXPECT_EQ(123, value.value().Int64OrDie()); + ASSERT_TRUE(value.value().IsUint64()); + EXPECT_EQ(123, value.value().Uint64OrDie()); } { diff --git a/test/extensions/filters/common/expr/evaluator_fuzz.proto b/test/extensions/filters/common/expr/evaluator_fuzz.proto index 5001d0fdae820..7b149e73eeb2e 100644 --- a/test/extensions/filters/common/expr/evaluator_fuzz.proto +++ b/test/extensions/filters/common/expr/evaluator_fuzz.proto @@ -6,7 +6,7 @@ import "google/api/expr/v1alpha1/syntax.proto"; import "test/fuzz/common.proto"; import "validate/validate.proto"; -// Structured input for rbac_fuzz_test. +// Structured input for fuzz test. message EvaluatorTestCase { google.api.expr.v1alpha1.Expr expression = 1 [(validate.rules).message.required = true]; From 225eaed65a7606d1146d4f139292aeed06d09cf1 Mon Sep 17 00:00:00 2001 From: Asra Ali Date: Mon, 28 Oct 2019 08:25:21 -0400 Subject: [PATCH 4/9] address comments Signed-off-by: Asra Ali --- bazel/repository_locations.bzl | 8 +- .../extensions/filters/common/expr/context.cc | 11 +- .../filters/common/expr/context_test.cc | 4 +- ...h-67e48e44650e25b93159729a7a4dd386625bb5c2 | 207 ++++++++++++++++++ .../common/expr/evaluator_fuzz_test.cc | 8 +- 5 files changed, 222 insertions(+), 16 deletions(-) create mode 100644 test/extensions/filters/common/expr/evaluator_corpus/crash-67e48e44650e25b93159729a7a4dd386625bb5c2 diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 9673ac3c41bd2..a4c6c895fc5ce 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -89,10 +89,10 @@ REPOSITORY_LOCATIONS = dict( urls = ["https://github.com/gabime/spdlog/archive/v1.3.1.tar.gz"], ), com_github_google_libprotobuf_mutator = dict( - sha256 = "f45c3ad82376d891cd0bcaa7165e83efd90e0014b00aebf0cbaf07eb05a1d3f9", - strip_prefix = "libprotobuf-mutator-d1fe8a7d8ae18f3d454f055eba5213c291986f21", - # 2019-07-10 - urls = ["https://github.com/google/libprotobuf-mutator/archive/d1fe8a7d8ae18f3d454f055eba5213c291986f21.tar.gz"], + sha256 = "54597f640c0ab5e5d783d2f3d3cfe8ad6da999ef1a194d89c2c5ab89a1fd8e13", + strip_prefix = "libprotobuf-mutator-dd89da92b59b1714bab6e2a135093948a1cf1c6a", + # 2019-10-08 + urls = ["https://github.com/google/libprotobuf-mutator/archive/dd89da92b59b1714bab6e2a135093948a1cf1c6a.tar.gz"], ), com_github_gperftools_gperftools = dict( # TODO(cmluciano): Bump to release 2.8 diff --git a/source/extensions/filters/common/expr/context.cc b/source/extensions/filters/common/expr/context.cc index a0d85da598b1c..c4de805c858fc 100644 --- a/source/extensions/filters/common/expr/context.cc +++ b/source/extensions/filters/common/expr/context.cc @@ -124,7 +124,7 @@ absl::optional ResponseWrapper::operator[](CelValue key) const { return CelValue::CreateInt64(code.value()); } } else if (value == Size) { - return CelValue::CreateUint64(info_.bytesSent()); + return CelValue::CreateInt64(info_.bytesSent()); } else if (value == Headers) { return CelValue::CreateMap(&headers_); } else if (value == Trailers) { @@ -186,13 +186,10 @@ absl::optional PeerWrapper::operator[](CelValue key) const { auto value = key.StringOrDie().value(); if (value == Address) { if (local_) { - if (info_.downstreamLocalAddress() != nullptr) { - return CelValue::CreateString(info_.downstreamLocalAddress()->asStringView()); - } + return CelValue::CreateString(info_.downstreamLocalAddress()->asStringView()); + } else { - if (info_.downstreamLocalAddress() != nullptr) { - return CelValue::CreateString(info_.downstreamRemoteAddress()->asStringView()); - } + return CelValue::CreateString(info_.downstreamRemoteAddress()->asStringView()); } } else if (value == Port) { if (local_) { diff --git a/test/extensions/filters/common/expr/context_test.cc b/test/extensions/filters/common/expr/context_test.cc index d14172556f1f6..a3b52cc89d1b9 100644 --- a/test/extensions/filters/common/expr/context_test.cc +++ b/test/extensions/filters/common/expr/context_test.cc @@ -209,8 +209,8 @@ TEST(Context, ResponseAttributes) { { auto value = response[CelValue::CreateString(Size)]; EXPECT_TRUE(value.has_value()); - ASSERT_TRUE(value.value().IsUint64()); - EXPECT_EQ(123, value.value().Uint64OrDie()); + ASSERT_TRUE(value.value().IsInt64()); + EXPECT_EQ(123, value.value().Int64OrDie()); } { diff --git a/test/extensions/filters/common/expr/evaluator_corpus/crash-67e48e44650e25b93159729a7a4dd386625bb5c2 b/test/extensions/filters/common/expr/evaluator_corpus/crash-67e48e44650e25b93159729a7a4dd386625bb5c2 new file mode 100644 index 0000000000000..eaf60aaa6d29d --- /dev/null +++ b/test/extensions/filters/common/expr/evaluator_corpus/crash-67e48e44650e25b93159729a7a4dd386625bb5c2 @@ -0,0 +1,207 @@ +expression { + id: 17179869184 + struct_expr { + entries { + map_key { + id: 27424471945274724 + list_expr { + } + } + } + entries { + map_key { + id: 27424471945274724 + list_expr { + elements { + id: 50331648 + } + } + } + } + entries { + map_key { + id: 27424471945274724 + list_expr { + elements { + id: 50331648 + } + } + } + } + entries { + map_key { + id: 27424471945274724 + list_expr { + elements { + id: 50331648 + } + } + } + } + entries { + map_key { + id: 27424471945274724 + list_expr { + elements { + select_expr { + operand { + ident_expr { + name: "\004\000\000\000" + } + } + } + } + } + } + } + entries { + map_key { + id: 27424471945274724 + list_expr { + elements { + id: 50331648 + } + } + } + } + entries { + map_key { + id: 27424471945274724 + list_expr { + elements { + id: 50331648 + } + } + } + } + entries { + map_key { + id: 27424471945274724 + } + value { + comprehension_expr { + iter_var: "\001\000\000\000\000\000\000\031" + } + } + } + } +} +request_headers { + headers { + key: "\t\000\000\000" + value: "&&" + } +} +response_headers { +} +trailers { + headers { + key: "\000\000\000\001" + value: "\000\000\000\001" + } + headers { + key: "\000\000\000\001" + value: "\000\000\000\001" + } + headers { + key: "\000\000\000\001" + value: "\000\000\000\001" + } + headers { + key: "\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177\177" + value: "\000\000\000\0010\000" + } +} +stream_info { + dynamic_metadata { + filter_metadata { + key: "" + value { + } + } + filter_metadata { + key: "" + value { + fields { + key: "(" + value { + } + } + } + } + filter_metadata { + key: "" + value { + fields { + key: "N5Envoy24ProtoValidation" + value { + } + } + } + } + filter_metadata { + key: "" + value { + fields { + key: "" + value { + number_value: 1.3262473693533e-315 + } + } + fields { + key: "(" + value { + } + } + } + } + filter_metadata { + key: "" + value { + fields { + key: "" + value { + number_value: 1.3262473693533e-315 + } + } + fields { + key: "(" + value { + } + } + } + } + filter_metadata { + key: "" + value { + fields { + key: "" + value { + number_value: 1.3262473693533e-315 + } + } + fields { + key: "(" + value { + } + } + } + } + } + response_code { + value: 134219776 + } + upstream_metadata { + filter_metadata { + key: "" + value { + fields { + key: "" + value { + number_value: 9.56944336513491e-315 + } + } + } + } + } +} diff --git a/test/extensions/filters/common/expr/evaluator_fuzz_test.cc b/test/extensions/filters/common/expr/evaluator_fuzz_test.cc index cef7ab7695152..b83ee598c6a95 100644 --- a/test/extensions/filters/common/expr/evaluator_fuzz_test.cc +++ b/test/extensions/filters/common/expr/evaluator_fuzz_test.cc @@ -41,13 +41,14 @@ TestStreamInfo makeStreamInfo(const test::fuzz::StreamInfo& info) { } DEFINE_PROTO_FUZZER(const test::extensions::filters::common::expr::EvaluatorTestCase& input) { + // Create builder without constant folding. + static Expr::BuilderPtr builder = Expr::createBuilder(nullptr); + try { // Validate that the input has an expression. TestUtility::validate(input); // Create the CEL expression. - Protobuf::Arena arena; - Expr::BuilderPtr builder = Expr::createBuilder(&arena); Expr::ExpressionPtr expr = Expr::createExpression(*builder, input.expression()); // Create the headers and stream_info to test against. @@ -57,7 +58,8 @@ DEFINE_PROTO_FUZZER(const test::extensions::filters::common::expr::EvaluatorTest Http::TestHeaderMapImpl response_trailers = Fuzz::fromHeaders(input.trailers()); // Evaluate the CEL expression. - Expr::evaluate(*expr, &arena, stream_info, &request_headers, &response_headers, + Protobuf::Arena arena; + Expr::evaluate(*expr, nullptr, stream_info, &request_headers, &response_headers, &response_trailers); } catch (const EnvoyException& e) { ENVOY_LOG_MISC(debug, "EnvoyException: {}", e.what()); From 4e42c3cfaced7bea448374617c4ddd7c8eda797b Mon Sep 17 00:00:00 2001 From: Asra Ali Date: Mon, 28 Oct 2019 15:11:49 -0400 Subject: [PATCH 5/9] add testcase Signed-off-by: Asra Ali --- ...h-d6a9858c9b8e8b60845af9f5adc9eaead58147bd | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 test/extensions/filters/common/expr/evaluator_corpus/crash-d6a9858c9b8e8b60845af9f5adc9eaead58147bd diff --git a/test/extensions/filters/common/expr/evaluator_corpus/crash-d6a9858c9b8e8b60845af9f5adc9eaead58147bd b/test/extensions/filters/common/expr/evaluator_corpus/crash-d6a9858c9b8e8b60845af9f5adc9eaead58147bd new file mode 100644 index 0000000000000..e8fd63ab0c8d7 --- /dev/null +++ b/test/extensions/filters/common/expr/evaluator_corpus/crash-d6a9858c9b8e8b60845af9f5adc9eaead58147bd @@ -0,0 +1,27 @@ +expression { + comprehension_expr { + iter_range { + ident_expr { + name: "request" + } + } + result { + id: 3530822107858468864 + } + } +} +trailers { + headers { + key: "\r\000" + } +} +stream_info { + dynamic_metadata { + filter_metadata { + key: "" + value { + } + } + } + requested_server_name: "/" +} From 3892a772644246ba8e592bf0677c1c0aec7bdc6a Mon Sep 17 00:00:00 2001 From: Asra Ali Date: Tue, 5 Nov 2019 08:57:06 -0500 Subject: [PATCH 6/9] fix new line Signed-off-by: Asra Ali --- source/extensions/filters/common/expr/context.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/source/extensions/filters/common/expr/context.cc b/source/extensions/filters/common/expr/context.cc index c4de805c858fc..b9f1f699e9045 100644 --- a/source/extensions/filters/common/expr/context.cc +++ b/source/extensions/filters/common/expr/context.cc @@ -187,7 +187,6 @@ absl::optional PeerWrapper::operator[](CelValue key) const { if (value == Address) { if (local_) { return CelValue::CreateString(info_.downstreamLocalAddress()->asStringView()); - } else { return CelValue::CreateString(info_.downstreamRemoteAddress()->asStringView()); } From 4104f46a8e471bb1ce603d454981358ab25e40e9 Mon Sep 17 00:00:00 2001 From: Asra Ali Date: Tue, 26 Nov 2019 11:03:51 -0500 Subject: [PATCH 7/9] address comments Signed-off-by: Asra Ali --- .../common/expr/evaluator_fuzz_test.cc | 24 +------------------ test/fuzz/BUILD | 1 + test/fuzz/common.proto | 2 ++ test/fuzz/utility.h | 5 +++- 4 files changed, 8 insertions(+), 24 deletions(-) diff --git a/test/extensions/filters/common/expr/evaluator_fuzz_test.cc b/test/extensions/filters/common/expr/evaluator_fuzz_test.cc index b83ee598c6a95..7969f2a9c0fcb 100644 --- a/test/extensions/filters/common/expr/evaluator_fuzz_test.cc +++ b/test/extensions/filters/common/expr/evaluator_fuzz_test.cc @@ -18,28 +18,6 @@ namespace Common { namespace Expr { namespace { -// Set stream_info using information from input. -// Duplicates most of fromStreamInfo, except is more lightweight without the use of mocks. -TestStreamInfo makeStreamInfo(const test::fuzz::StreamInfo& info) { - TestStreamInfo stream_info; - // TODO(asraa): Set upstreamHost() and downstreamSslConnection(). - stream_info.metadata_ = info.dynamic_metadata(); - const auto start_time = - static_cast(std::numeric_limits::max()) < - info.start_time() - ? 0 - : info.start_time() / 1000; - stream_info.start_time_ = SystemTime(std::chrono::microseconds(start_time)); - stream_info.setRequestedServerName(info.requested_server_name()); - stream_info.response_code_ = info.has_response_code() ? info.response_code().value() : 200; - auto address = Network::Utility::resolveUrl("tcp://10.0.0.1:443"); - stream_info.upstream_local_address_ = address; - stream_info.downstream_local_address_ = address; - stream_info.downstream_direct_remote_address_ = address; - stream_info.downstream_remote_address_ = address; - return stream_info; -} - DEFINE_PROTO_FUZZER(const test::extensions::filters::common::expr::EvaluatorTestCase& input) { // Create builder without constant folding. static Expr::BuilderPtr builder = Expr::createBuilder(nullptr); @@ -52,7 +30,7 @@ DEFINE_PROTO_FUZZER(const test::extensions::filters::common::expr::EvaluatorTest Expr::ExpressionPtr expr = Expr::createExpression(*builder, input.expression()); // Create the headers and stream_info to test against. - TestStreamInfo stream_info = makeStreamInfo(input.stream_info()); + TestStreamInfo stream_info = Fuzz::fromStreamInfo(input.stream_info()); Http::TestHeaderMapImpl request_headers = Fuzz::fromHeaders(input.request_headers()); Http::TestHeaderMapImpl response_headers = Fuzz::fromHeaders(input.response_headers()); Http::TestHeaderMapImpl response_trailers = Fuzz::fromHeaders(input.trailers()); diff --git a/test/fuzz/BUILD b/test/fuzz/BUILD index 3763795e59104..44507b1c1a554 100644 --- a/test/fuzz/BUILD +++ b/test/fuzz/BUILD @@ -51,6 +51,7 @@ envoy_cc_test_library( deps = [ ":common_proto_cc_proto", "//source/common/common:empty_string", + "//source/common/network:resolver_lib", "//source/common/network:utility_lib", "//test/common/stream_info:test_util", "//test/mocks/ssl:ssl_mocks", diff --git a/test/fuzz/common.proto b/test/fuzz/common.proto index 073f5b30e3244..ce042d39c58f5 100644 --- a/test/fuzz/common.proto +++ b/test/fuzz/common.proto @@ -3,6 +3,7 @@ syntax = "proto3"; package test.fuzz; import "envoy/api/v2/core/base.proto"; +import "envoy/api/v2/core/address.proto"; import "google/protobuf/wrappers.proto"; @@ -18,4 +19,5 @@ message StreamInfo { google.protobuf.UInt32Value response_code = 3; envoy.api.v2.core.Metadata upstream_metadata = 4; string requested_server_name = 5; + envoy.api.v2.core.Address address = 6; } diff --git a/test/fuzz/utility.h b/test/fuzz/utility.h index 421a031aea57a..84d24eea52c45 100644 --- a/test/fuzz/utility.h +++ b/test/fuzz/utility.h @@ -1,6 +1,7 @@ #pragma once #include "common/common/empty_string.h" +#include "common/network/resolver_impl.h" #include "common/network/utility.h" #include "test/common/stream_info/test_util.h" @@ -112,6 +113,7 @@ const std::string TestSubjectPeer = inline TestStreamInfo fromStreamInfo(const test::fuzz::StreamInfo& stream_info) { // Set mocks' default string return value to be an empty string. + // TODO(asraa): Speed up this function, which is slowed because of the use of mocks. testing::DefaultValue::Set(EMPTY_STRING); TestStreamInfo test_stream_info; test_stream_info.metadata_ = stream_info.dynamic_metadata(); @@ -125,12 +127,13 @@ inline TestStreamInfo fromStreamInfo(const test::fuzz::StreamInfo& stream_info) if (stream_info.has_response_code()) { test_stream_info.response_code_ = stream_info.response_code().value(); } + test_stream_info.setRequestedServerName(stream_info.requested_server_name()); auto upstream_host = std::make_shared>(); auto upstream_metadata = std::make_shared( replaceInvalidStringValues(stream_info.upstream_metadata())); ON_CALL(*upstream_host, metadata()).WillByDefault(testing::Return(upstream_metadata)); test_stream_info.upstream_host_ = upstream_host; - auto address = Network::Utility::resolveUrl("tcp://10.0.0.1:443"); + auto address = Envoy::Network::Address::resolveProtoAddress(stream_info.address()); test_stream_info.upstream_local_address_ = address; test_stream_info.downstream_local_address_ = address; test_stream_info.downstream_direct_remote_address_ = address; From e0836fc2173dd1df1892965376ec6bb3d9863c74 Mon Sep 17 00:00:00 2001 From: Asra Ali Date: Tue, 3 Dec 2019 09:52:37 -0500 Subject: [PATCH 8/9] make cel exception Signed-off-by: Asra Ali --- .../filters/common/expr/evaluator.cc | 4 ++-- .../filters/common/expr/evaluator.h | 6 ++++++ ...h-87e3c780acf4403ddd8b182496e6cad5ac5efd66 | 6 ++++++ .../common/expr/evaluator_fuzz_test.cc | 21 ++++++++++++------- test/fuzz/common.proto | 2 ++ test/fuzz/utility.h | 13 +++++++----- 6 files changed, 37 insertions(+), 15 deletions(-) create mode 100644 test/extensions/filters/common/expr/evaluator_corpus/crash-87e3c780acf4403ddd8b182496e6cad5ac5efd66 diff --git a/source/extensions/filters/common/expr/evaluator.cc b/source/extensions/filters/common/expr/evaluator.cc index 4d04bf534c0ed..dff584d3ee9e7 100644 --- a/source/extensions/filters/common/expr/evaluator.cc +++ b/source/extensions/filters/common/expr/evaluator.cc @@ -49,7 +49,7 @@ BuilderPtr createBuilder(Protobuf::Arena* arena) { auto register_status = google::api::expr::runtime::RegisterBuiltinFunctions(builder->GetRegistry(), options); if (!register_status.ok()) { - throw EnvoyException( + throw CelException( absl::StrCat("failed to register built-in functions: ", register_status.message())); } return builder; @@ -59,7 +59,7 @@ ExpressionPtr createExpression(Builder& builder, const google::api::expr::v1alph google::api::expr::v1alpha1::SourceInfo source_info; auto cel_expression_status = builder.CreateExpression(&expr, &source_info); if (!cel_expression_status.ok()) { - throw EnvoyException( + throw CelException( absl::StrCat("failed to create an expression: ", cel_expression_status.status().message())); } return std::move(cel_expression_status.ValueOrDie()); diff --git a/source/extensions/filters/common/expr/evaluator.h b/source/extensions/filters/common/expr/evaluator.h index 39812c60a3437..16cdada5953df 100644 --- a/source/extensions/filters/common/expr/evaluator.h +++ b/source/extensions/filters/common/expr/evaluator.h @@ -52,6 +52,12 @@ absl::optional evaluate(const Expression& expr, Protobuf::Arena* arena bool matches(const Expression& expr, const StreamInfo::StreamInfo& info, const Http::HeaderMap& headers); +// Thrown when there is an CEL library error. +class CelException : public EnvoyException { +public: + CelException(const std::string& what) : EnvoyException(what) {} +}; + } // namespace Expr } // namespace Common } // namespace Filters diff --git a/test/extensions/filters/common/expr/evaluator_corpus/crash-87e3c780acf4403ddd8b182496e6cad5ac5efd66 b/test/extensions/filters/common/expr/evaluator_corpus/crash-87e3c780acf4403ddd8b182496e6cad5ac5efd66 new file mode 100644 index 0000000000000..33e49e8b02c95 --- /dev/null +++ b/test/extensions/filters/common/expr/evaluator_corpus/crash-87e3c780acf4403ddd8b182496e6cad5ac5efd66 @@ -0,0 +1,6 @@ +trailers { +} +stream_info { + address { + } +} diff --git a/test/extensions/filters/common/expr/evaluator_fuzz_test.cc b/test/extensions/filters/common/expr/evaluator_fuzz_test.cc index 7969f2a9c0fcb..6374e6e9f04a2 100644 --- a/test/extensions/filters/common/expr/evaluator_fuzz_test.cc +++ b/test/extensions/filters/common/expr/evaluator_fuzz_test.cc @@ -25,22 +25,27 @@ DEFINE_PROTO_FUZZER(const test::extensions::filters::common::expr::EvaluatorTest try { // Validate that the input has an expression. TestUtility::validate(input); + } catch (const EnvoyException& e) { + ENVOY_LOG_MISC(debug, "EnvoyException: {}", e.what()); + return; + } + + // Create the headers and stream_info to test against. + TestStreamInfo stream_info = Fuzz::fromStreamInfo(input.stream_info()); + Http::TestHeaderMapImpl request_headers = Fuzz::fromHeaders(input.request_headers()); + Http::TestHeaderMapImpl response_headers = Fuzz::fromHeaders(input.response_headers()); + Http::TestHeaderMapImpl response_trailers = Fuzz::fromHeaders(input.trailers()); + try { // Create the CEL expression. Expr::ExpressionPtr expr = Expr::createExpression(*builder, input.expression()); - // Create the headers and stream_info to test against. - TestStreamInfo stream_info = Fuzz::fromStreamInfo(input.stream_info()); - Http::TestHeaderMapImpl request_headers = Fuzz::fromHeaders(input.request_headers()); - Http::TestHeaderMapImpl response_headers = Fuzz::fromHeaders(input.response_headers()); - Http::TestHeaderMapImpl response_trailers = Fuzz::fromHeaders(input.trailers()); - // Evaluate the CEL expression. Protobuf::Arena arena; Expr::evaluate(*expr, nullptr, stream_info, &request_headers, &response_headers, &response_trailers); - } catch (const EnvoyException& e) { - ENVOY_LOG_MISC(debug, "EnvoyException: {}", e.what()); + } catch (const CelException& e) { + ENVOY_LOG_MISC(debug, "CelException: {}", e.what()); } } diff --git a/test/fuzz/common.proto b/test/fuzz/common.proto index ce042d39c58f5..dcd1ccea15975 100644 --- a/test/fuzz/common.proto +++ b/test/fuzz/common.proto @@ -7,6 +7,8 @@ import "envoy/api/v2/core/address.proto"; import "google/protobuf/wrappers.proto"; +import "validate/validate.proto"; + // Common fuzzing input types. message Headers { diff --git a/test/fuzz/utility.h b/test/fuzz/utility.h index 84d24eea52c45..8fd3df4084158 100644 --- a/test/fuzz/utility.h +++ b/test/fuzz/utility.h @@ -133,11 +133,14 @@ inline TestStreamInfo fromStreamInfo(const test::fuzz::StreamInfo& stream_info) replaceInvalidStringValues(stream_info.upstream_metadata())); ON_CALL(*upstream_host, metadata()).WillByDefault(testing::Return(upstream_metadata)); test_stream_info.upstream_host_ = upstream_host; - auto address = Envoy::Network::Address::resolveProtoAddress(stream_info.address()); - test_stream_info.upstream_local_address_ = address; - test_stream_info.downstream_local_address_ = address; - test_stream_info.downstream_direct_remote_address_ = address; - test_stream_info.downstream_remote_address_ = address; + if (stream_info.has_address()) { + ENVOY_LOG_MISC(info, "has address {}", stream_info.address().DebugString()); + auto address = Envoy::Network::Address::resolveProtoAddress(stream_info.address()); + test_stream_info.upstream_local_address_ = address; + test_stream_info.downstream_local_address_ = address; + test_stream_info.downstream_direct_remote_address_ = address; + test_stream_info.downstream_remote_address_ = address; + } auto connection_info = std::make_shared>(); ON_CALL(*connection_info, subjectPeerCertificate()) .WillByDefault(testing::ReturnRef(TestSubjectPeer)); From 0d2fb609e0b4161e2b725d598c3587e5034a98d7 Mon Sep 17 00:00:00 2001 From: Asra Ali Date: Tue, 3 Dec 2019 10:59:30 -0500 Subject: [PATCH 9/9] fix address not set Signed-off-by: Asra Ali --- test/fuzz/utility.h | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/test/fuzz/utility.h b/test/fuzz/utility.h index 8fd3df4084158..62771bd42b484 100644 --- a/test/fuzz/utility.h +++ b/test/fuzz/utility.h @@ -133,14 +133,13 @@ inline TestStreamInfo fromStreamInfo(const test::fuzz::StreamInfo& stream_info) replaceInvalidStringValues(stream_info.upstream_metadata())); ON_CALL(*upstream_host, metadata()).WillByDefault(testing::Return(upstream_metadata)); test_stream_info.upstream_host_ = upstream_host; - if (stream_info.has_address()) { - ENVOY_LOG_MISC(info, "has address {}", stream_info.address().DebugString()); - auto address = Envoy::Network::Address::resolveProtoAddress(stream_info.address()); - test_stream_info.upstream_local_address_ = address; - test_stream_info.downstream_local_address_ = address; - test_stream_info.downstream_direct_remote_address_ = address; - test_stream_info.downstream_remote_address_ = address; - } + auto address = stream_info.has_address() + ? Envoy::Network::Address::resolveProtoAddress(stream_info.address()) + : Network::Utility::resolveUrl("tcp://10.0.0.1:443"); + test_stream_info.upstream_local_address_ = address; + test_stream_info.downstream_local_address_ = address; + test_stream_info.downstream_direct_remote_address_ = address; + test_stream_info.downstream_remote_address_ = address; auto connection_info = std::make_shared>(); ON_CALL(*connection_info, subjectPeerCertificate()) .WillByDefault(testing::ReturnRef(TestSubjectPeer));