utility: fix strftime overflow handling. (envoyproxy#4321)

Existing strftime uses did not correctly handle buffer overflow conditions, where strftime returns 0 and the buffer contents are undefined. This was discovered by an internal equivalent of oss-fuzz. Risk level: Low Testing: Unit test and corpus entry added. Signed-off-by: Harvey Tuch <htuch@google.com>
rshriram · Aug 31, 2018 · 00ffe44 · 00ffe44
1 parent af1183c
commit 00ffe44
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 3 deletions.
diff --git a/source/common/common/utility.cc b/source/common/common/utility.cc
@@ -167,8 +167,8 @@ std::string DateFormatter::fromTime(time_t time) const {
   gmtime_r(&time, &current_tm);
 
   std::array<char, 1024> buf;
-  strftime(&buf[0], buf.size(), format_string_.c_str(), &current_tm);
-  return std::string(&buf[0]);
+  const size_t len = strftime(&buf[0], buf.size(), format_string_.c_str(), &current_tm);
+  return std::string(&buf[0], len);
 }
 
 std::string
@@ -185,7 +185,7 @@ DateFormatter::fromTimeAndPrepareSpecifierOffsets(time_t time, SpecifierOffsets&
   for (const auto& specifier : specifiers_) {
     const size_t formatted_length =
         strftime(&buf[0], buf.size(), specifier.segment_.c_str(), &current_tm);
-    absl::StrAppend(&formatted, &buf[0],
+    absl::StrAppend(&formatted, absl::string_view(&buf[0], formatted_length),
                     specifier.second_ ? seconds_str : std::string(specifier.width_, '?'));
 
     // This computes and saves offset of each specifier's pattern to correct its position after the

diff --git a/test/common/common/utility_test.cc b/test/common/common/utility_test.cc
@@ -807,4 +807,15 @@ TEST(WelfordStandardDeviation, InsufficientData) {
   EXPECT_TRUE(std::isnan(wsd.computeStandardDeviation()));
 }
 
+TEST(DateFormatter, FromTime) {
+  const SystemTime time1(std::chrono::seconds(1522796769));
+  EXPECT_EQ("2018-04-03T23:06:09.000Z", DateFormatter("%Y-%m-%dT%H:%M:%S.000Z").fromTime(time1));
+  EXPECT_EQ("aaa23", DateFormatter(std::string(3, 'a') + "%H").fromTime(time1));
+  EXPECT_EQ("", DateFormatter(std::string(1022, 'a') + "%H").fromTime(time1));
+  const time_t time2 = 0;
+  EXPECT_EQ("1970-01-01T00:00:00.000Z", DateFormatter("%Y-%m-%dT%H:%M:%S.000Z").fromTime(time2));
+  EXPECT_EQ("aaa00", DateFormatter(std::string(3, 'a') + "%H").fromTime(time2));
+  EXPECT_EQ("", DateFormatter(std::string(1022, 'a') + "%H").fromTime(time2));
+}
+
 } // namespace Envoy
diff --git a/test/common/router/header_parser_corpus/foo b/test/common/router/header_parser_corpus/foo
@@ -0,0 +1,16 @@
+headers_to_add {
+  header {
+    key: "foo"
+    value: "%START_TIME(%0\240&&&&&&&&zzzzzzzzzzzzzamA(24d\240\240\240\240\240\240\240\240Q240\240\240\240\240\020\240^240&&7&&&&&&&&&&\006&val\177\377\376&&aenam\001s %1f,  %85/5_inf %8,,,,,,,,,,,,t_timefo 5#5555#555.5,  %85/5_inf %8,,,,,,,,,,,,t_t  %85555555\2005  %85/5555Fme:\t15227 f-5555_inf 965L5$59f)%"
+  }
+}
+headers_to_remove: "7"
+request_info {
+  upstream_metadata {
+    filter_metadata {
+      key: ""
+      value {
+      }
+    }
+  }
+}