Don't read beyond a StringPiece's bounds in StringToNumber()

This change was included in
https://chromium-review.googlesource.com/c/chromium/src/+/947745
and was reverted by
https://chromium-review.googlesource.com/c/chromium/src/+/950343
but is still needed by
https://chromium-review.googlesource.com/c/chromium/src/+/945049

Bug: chromium:817982, chromium:818376
Change-Id: I4f97d632ba9f86c17d3df55c404c0ae14e1d39c9
Reviewed-on: https://chromium-review.googlesource.com/950466Reviewed-by: Chris Palmer <palmer@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541183}

third_party/crashpad/README.chromium

@@ -37,3 +37,9 @@ $ git am --3way --message-id -p4 /tmp/patchdir
 Local Modifications:
  - codereview.settings has been excluded.
  - thread_log_messages.cc (using ThreadLocalStorage::Slot instead of StaticSlot)
+ - util/linux/memory_map.cc
+ - util/mach/symbolic_constants_mach.cc
+ - util/posix/symbolic_constants_posix.cc
+ - util/stdlib/string_number_conversion.h
+ - util/stdlib/string_number_conversion.cc
+ - util/stdlib/string_number_conversion_test.cc

third_party/crashpad/crashpad/util/linux/memory_map.cc

@@ -36,7 +36,7 @@ namespace {
 // Simply adding a StringToNumber for longs doesn't work since sometimes long
 // and int64_t are actually the same type, resulting in a redefinition error.
 template <typename Type>
-bool LocalStringToNumber(const base::StringPiece& string, Type* number) {
+bool LocalStringToNumber(const std::string& string, Type* number) {
   static_assert(sizeof(Type) == sizeof(int) || sizeof(Type) == sizeof(int64_t),
                 "Unexpected Type size");
 

third_party/crashpad/crashpad/util/mach/symbolic_constants_mach.cc

@@ -239,7 +239,8 @@ bool StringToException(const base::StringPiece& string,
   }
 
   if (options & kAllowNumber) {
-    return StringToNumber(string, reinterpret_cast<unsigned int*>(exception));
+    return StringToNumber(std::string(string.data(), string.length()),
+                          reinterpret_cast<unsigned int*>(exception));
   }
 
   return false;
@@ -352,7 +353,7 @@ bool StringToExceptionMask(const base::StringPiece& string,
   }
 
   if (options & kAllowNumber) {
-    return StringToNumber(string,
+    return StringToNumber(std::string(string.data(), string.length()),
                           reinterpret_cast<unsigned int*>(exception_mask));
   }
 
@@ -452,7 +453,8 @@ bool StringToExceptionBehavior(const base::StringPiece& string,
 
   if (options & kAllowNumber) {
     exception_behavior_t temp_behavior;
-    if (!StringToNumber(sp, reinterpret_cast<unsigned int*>(&temp_behavior))) {
+    if (!StringToNumber(std::string(sp.data(), sp.length()),
+                        reinterpret_cast<unsigned int*>(&temp_behavior))) {
       return false;
     }
     build_behavior |= temp_behavior;
@@ -539,7 +541,8 @@ bool StringToThreadStateFlavor(const base::StringPiece& string,
   }
 
   if (options & kAllowNumber) {
-    return StringToNumber(string, reinterpret_cast<unsigned int*>(flavor));
+    return StringToNumber(std::string(string.data(), string.length()),
+                          reinterpret_cast<unsigned int*>(flavor));
   }
 
   return false;

third_party/crashpad/crashpad/util/posix/symbolic_constants_posix.cc

@@ -161,7 +161,7 @@ bool StringToSignal(const base::StringPiece& string,
   }
 
   if (options & kAllowNumber) {
-    return StringToNumber(string, signal);
+    return StringToNumber(std::string(string.data(), string.length()), signal);
   }
 
   return false;

third_party/crashpad/crashpad/util/stdlib/string_number_conversion.cc

@@ -116,7 +116,7 @@ struct StringToUnsignedInt64Traits
 };
 
 template <typename Traits>
-bool StringToIntegerInternal(const base::StringPiece& string,
+bool StringToIntegerInternal(const std::string& string,
                              typename Traits::IntType* number) {
   using IntType = typename Traits::IntType;
   using LongType = typename Traits::LongType;
@@ -127,14 +127,6 @@ bool StringToIntegerInternal(const base::StringPiece& string,
     return false;
   }
 
-  if (string[string.length()] != '\0') {
-    // The implementations use the C standard library’s conversion routines,
-    // which rely on the strings having a trailing NUL character. std::string
-    // will NUL-terminate.
-    std::string terminated_string(string.data(), string.length());
-    return StringToIntegerInternal<Traits>(terminated_string, number);
-  }
-
   errno = 0;
   char* end;
   LongType result = Traits::Convert(string.data(), &end, 0);
@@ -152,19 +144,19 @@ bool StringToIntegerInternal(const base::StringPiece& string,
 
 namespace crashpad {
 
-bool StringToNumber(const base::StringPiece& string, int* number) {
+bool StringToNumber(const std::string& string, int* number) {
   return StringToIntegerInternal<StringToIntTraits>(string, number);
 }
 
-bool StringToNumber(const base::StringPiece& string, unsigned int* number) {
+bool StringToNumber(const std::string& string, unsigned int* number) {
   return StringToIntegerInternal<StringToUnsignedIntTraits>(string, number);
 }
 
-bool StringToNumber(const base::StringPiece& string, int64_t* number) {
+bool StringToNumber(const std::string& string, int64_t* number) {
   return StringToIntegerInternal<StringToInt64Traits>(string, number);
 }
 
-bool StringToNumber(const base::StringPiece& string, uint64_t* number) {
+bool StringToNumber(const std::string& string, uint64_t* number) {
   return StringToIntegerInternal<StringToUnsignedInt64Traits>(string, number);
 }
 

third_party/crashpad/crashpad/util/stdlib/string_number_conversion.h

@@ -15,7 +15,7 @@
 #ifndef CRASHPAD_UTIL_STDLIB_STRING_NUMBER_CONVERSION_H_
 #define CRASHPAD_UTIL_STDLIB_STRING_NUMBER_CONVERSION_H_
 
-#include "base/strings/string_piece.h"
+#include <string>
 
 namespace crashpad {
 
@@ -54,10 +54,10 @@ namespace crashpad {
 //!     allow arbitrary bases based on whether the string begins with a prefix
 //!     indicating its base. The functions here are provided for situations
 //!     where such prefix recognition is desirable.
-bool StringToNumber(const base::StringPiece& string, int* number);
-bool StringToNumber(const base::StringPiece& string, unsigned int* number);
-bool StringToNumber(const base::StringPiece& string, int64_t* number);
-bool StringToNumber(const base::StringPiece& string, uint64_t* number);
+bool StringToNumber(const std::string& string, int* number);
+bool StringToNumber(const std::string& string, unsigned int* number);
+bool StringToNumber(const std::string& string, int64_t* number);
+bool StringToNumber(const std::string& string, uint64_t* number);
 //! \}
 
 }  // namespace crashpad

third_party/crashpad/crashpad/util/stdlib/string_number_conversion_test.cc

@@ -114,13 +114,9 @@ TEST(StringNumberConversion, StringToInt) {
   // is split to avoid MSVC warning:
   //   "decimal digit terminates octal escape sequence".
   static constexpr char input[] = "6\000" "6";
-  base::StringPiece input_string(input, arraysize(input) - 1);
+  std::string input_string(input, arraysize(input) - 1);
   int output;
   EXPECT_FALSE(StringToNumber(input_string, &output));
-
-  // Ensure that a NUL is not required at the end of the string.
-  EXPECT_TRUE(StringToNumber(base::StringPiece("66", 1), &output));
-  EXPECT_EQ(output, 6);
 }
 
 TEST(StringNumberConversion, StringToUnsignedInt) {
@@ -212,13 +208,9 @@ TEST(StringNumberConversion, StringToUnsignedInt) {
   // is split to avoid MSVC warning:
   //   "decimal digit terminates octal escape sequence".
   static constexpr char input[] = "6\000" "6";
-  base::StringPiece input_string(input, arraysize(input) - 1);
+  std::string input_string(input, arraysize(input) - 1);
   unsigned int output;
   EXPECT_FALSE(StringToNumber(input_string, &output));
-
-  // Ensure that a NUL is not required at the end of the string.
-  EXPECT_TRUE(StringToNumber(base::StringPiece("66", 1), &output));
-  EXPECT_EQ(output, 6u);
 }
 
 TEST(StringNumberConversion, StringToInt64) {