fuzz: fixes oss-fuzz: 8363 (#3905)

oss-fuzz issue (8363): https://oss-fuzz.com/v2/testcase-detail/5988544525893632 The crash was because of passing nan to Envoy::ProtobufPercentHelper::convertPercent, it asserts since it is not in the numeric range. Instead of adding a check in this function, have added a check in the preprocessor so that it goes to checkAndReturnDefault and the default value is used. Have also added the crashing testcase to the corpus. Risk Level: Low Testing: Tested unit tests (bazel test //server:server_fuzz_test), built and ran fuzzers with oss-fuzz. Signed-off-by: Anirudh M <m.anirudh18@gmail.com>
envoyproxy · Jul 25, 2018 · 8459237 · 8459237
1 parent ec0179a
commit 8459237
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 3 deletions.
diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h
@@ -71,11 +71,16 @@ uint64_t fractionalPercentDenominatorToInt(const envoy::type::FractionalPercent&
 // @param field_name supplies the field name in the message.
 // @param max_value supplies the maximum allowed integral value (e.g., 100, 10000, etc.).
 // @param default_value supplies the default if the field is not present.
+//
+// TODO(anirudhmurali): Recommended to capture and validate NaN values in PGV
+// Issue: https://github.com/lyft/protoc-gen-validate/issues/85
 #define PROTOBUF_PERCENT_TO_ROUNDED_INTEGER_OR_DEFAULT(message, field_name, max_value,             \
                                                        default_value)                              \
-  ((message).has_##field_name()                                                                    \
-       ? ProtobufPercentHelper::convertPercent((message).field_name().value(), max_value)          \
-       : ProtobufPercentHelper::checkAndReturnDefault(default_value, max_value))
+  (!std::isnan((message).field_name().value())                                                     \
+       ? (message).has_##field_name()                                                              \
+             ? ProtobufPercentHelper::convertPercent((message).field_name().value(), max_value)    \
+             : ProtobufPercentHelper::checkAndReturnDefault(default_value, max_value)              \
+       : throw EnvoyException(fmt::format("Value not in the range of 0..100 range.")))
 
 namespace Envoy {
 

diff --git a/test/common/protobuf/utility_test.cc b/test/common/protobuf/utility_test.cc
@@ -13,6 +13,15 @@
 
 namespace Envoy {
 
+TEST(UtilityTest, convertPercentNaN) {
+  envoy::api::v2::Cluster::CommonLbConfig common_config_;
+  common_config_.mutable_healthy_panic_threshold()->set_value(
+      std::numeric_limits<double>::quiet_NaN());
+  EXPECT_THROW(PROTOBUF_PERCENT_TO_ROUNDED_INTEGER_OR_DEFAULT(common_config_,
+                                                              healthy_panic_threshold, 100, 50),
+               EnvoyException);
+}
+
 TEST(UtilityTest, RepeatedPtrUtilDebugString) {
   Protobuf::RepeatedPtrField<ProtobufWkt::UInt32Value> repeated;
   EXPECT_EQ("[]", RepeatedPtrUtil::debugString(repeated));

diff --git a/test/server/server_corpus/clusterfuzz-testcase-server_fuzz_test-5988544525893632 b/test/server/server_corpus/clusterfuzz-testcase-server_fuzz_test-5988544525893632
@@ -0,0 +1,29 @@
+static_resources {
+  clusters {
+    name: "-2353373969551157135775236"
+    connect_timeout {
+      seconds: 12884901890
+    }
+    hosts {
+      pipe {
+        path: "@"
+      }
+    }
+    outlier_detection {
+    }
+    common_lb_config {
+      healthy_panic_threshold {
+        value: nan
+      }
+    }
+  }
+}
+admin {
+  access_log_path: "@r"
+  address {
+    pipe {
+      path: "W"
+    }
+  }
+}
+