From 845923714078e5a3528e85d4b58f2cc95850a1c6 Mon Sep 17 00:00:00 2001
From: Anirudh <m.anirudh18@gmail.com>
Date: Wed, 25 Jul 2018 23:28:44 +0530
Subject: [PATCH] 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>
---
 source/common/protobuf/utility.h              | 11 +++++--
 test/common/protobuf/utility_test.cc          |  9 ++++++
 ...testcase-server_fuzz_test-5988544525893632 | 29 +++++++++++++++++++
 3 files changed, 46 insertions(+), 3 deletions(-)
 create mode 100644 test/server/server_corpus/clusterfuzz-testcase-server_fuzz_test-5988544525893632

diff --git a/source/common/protobuf/utility.h b/source/common/protobuf/utility.h
index 9229d6eebcce..93d098c75d73 100644
--- 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
index e7f5ee34fd40..15dbfa1a55f5 100644
--- 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
new file mode 100644
index 000000000000..5c8b2ec2c49e
--- /dev/null
+++ 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"
+    }
+  }
+}
+