Fix potential buffer over-read errors for un-terminated JSON strings and comments.

base/json/json_parser_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/json/json_parser.h"
 
 #include <stddef.h>
 
 #include <memory>
 
@@ skipping 10 matching lines @@
  public:
   JSONParser* NewTestParser(const std::string& input,
                             int options = JSON_PARSE_RFC) {
     JSONParser* parser = new JSONParser(options);
     parser->start_pos_ = input.data();
     parser->pos_ = parser->start_pos_;
     parser->end_pos_ = parser->start_pos_ + input.length();
     return parser;
   }
 
+  // MSan will do a better job detecting over-read errors if the input is
+  // not nul-terminated on the heap. This will copy |input| to a new buffer
+  // owned by |owner|, returning a StringPiece to |owner|.
+  StringPiece MakeNotNullTerminatedInput(const char* input,
+                                         std::unique_ptr<char[]>* owner) {
+    size_t str_len = strlen(input);
+    owner->reset(new char[str_len]);
+    memcpy(owner->get(), input, str_len);
+    return StringPiece(owner->get(), str_len);
+  }
+
   void TestLastThree(JSONParser* parser) {
     EXPECT_EQ(',', *parser->NextChar());
     EXPECT_EQ('|', *parser->NextChar());
     EXPECT_EQ('\0', *parser->NextChar());
     EXPECT_EQ(parser->end_pos_, parser->pos_);
   }
 };
 
 TEST_F(JSONParserTest, NextChar) {
   std::string input("Hello world");
@@ skipping 319 matching lines @@
       {"9e-3", true, 0.009},
       {"2e+", false, 0},
       {"2e+2", true, 200},
       // clang-format on
   };
 
   for (unsigned int i = 0; i < arraysize(kCases); ++i) {
     auto test_case = kCases[i];
     SCOPED_TRACE(StringPrintf("case %u: \"%s\"", i, test_case.input));
 
-    // MSan will do a better job detecting over-read errors if the input is
-    // not nul-terminated on the heap.
-    size_t str_len = strlen(test_case.input);
-    auto non_nul_termianted = MakeUnique<char[]>(str_len);
-    memcpy(non_nul_termianted.get(), test_case.input, str_len);
+    std::unique_ptr<char[]> input_owner;
+    StringPiece input =
+        MakeNotNullTerminatedInput(test_case.input, &input_owner);
 
-    StringPiece string_piece(non_nul_termianted.get(), str_len);
-    std::unique_ptr<Value> result = JSONReader::Read(string_piece);
+    std::unique_ptr<Value> result = JSONReader::Read(input);
     if (test_case.parse_success) {
       EXPECT_TRUE(result);
     } else {
       EXPECT_FALSE(result);
     }
 
     if (!result)
       continue;
 
     double double_value = 0;
     EXPECT_TRUE(result->GetAsDouble(&double_value));
     EXPECT_EQ(test_case.value, double_value);
   }
 }
 
+TEST_F(JSONParserTest, UnterminatedInputs) {
+  const char* kCases[] = {
+      // clang-format off
+      "/",
+      "//",
+      "/*",
+      "\"xxxxxx",
+      "\"",
+      "{   ",
+      "[\t",
+      "tru",
+      "fals",
+      "nul",
+      "\"\\x2",
+      "\"\\u123",
+      // clang-format on
+  };
+
+  for (unsigned int i = 0; i < arraysize(kCases); ++i) {

[brettw, 2017/05/09 19:53:14]

Random optional thought: Does C++11 let you do:
  for (const char* test_case : kCases)
here? I thought begin() and end() worked for static arrays. Maybe I'm dreaming.

[Robert Sesek, 2017/05/09 21:27:36]

The |for (:)| construct does work, but I use |i| in the SCOPED_TRACE.

+    auto* test_case = kCases[i];
+    SCOPED_TRACE(StringPrintf("case %u: \"%s\"", i, test_case));
+
+    std::unique_ptr<char[]> input_owner;
+    StringPiece input = MakeNotNullTerminatedInput(test_case, &input_owner);
+
+    EXPECT_FALSE(JSONReader::Read(input));
+  }
+}
+
 }  // namespace internal
 }  // namespace base