Fix a couple of integer issues in Pickle deserialization. Neither represent

a significant risk because the code is not directly exposed to user input. In
addition, neither error leads to memory corruption. At worse, there's a C++
exception or abort().

BUG=NONE
TEST=PickleTest.EvilLengths

Review URL: http://codereview.chromium.org/146121

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@19249 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 32 insertions, 1 deletions

diff --git a/base/pickle.cc b/base/pickle.cc
index 5e249c7..c3df8bc 100644
--- a/base/pickle.cc
+++ b/base/pickle.cc
@@ -208,6 +208,9 @@ bool Pickle::ReadWString(void** iter, std::wstring* result) const {
   int len;
   if (!ReadLength(iter, &len))
     return false;
+  // Avoid integer overflow.
+  if (len > INT_MAX / static_cast<int>(sizeof(wchar_t)))
+    return false;
   if (!IteratorHasRoomFor(*iter, len * sizeof(wchar_t)))
     return false;
 
@@ -224,7 +227,7 @@ bool Pickle::ReadString16(void** iter, string16* result) const {
   int len;
   if (!ReadLength(iter, &len))
     return false;
-  if (!IteratorHasRoomFor(*iter, len))
+  if (!IteratorHasRoomFor(*iter, len * sizeof(char16)))
     return false;
 
   char16* chars = reinterpret_cast<char16*>(*iter);
diff --git a/base/pickle_unittest.cc b/base/pickle_unittest.cc
index 48ab1cd..4b8373c 100644
--- a/base/pickle_unittest.cc
+++ b/base/pickle_unittest.cc
@@ -7,6 +7,7 @@
 #include "base/basictypes.h"
 #include "base/pickle.h"
 #include "base/scoped_ptr.h"
+#include "base/string16.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -218,3 +219,30 @@ TEST(PickleTest, EqualsOperator) {
   copy = copy_refs_source_buffer;
   ASSERT_EQ(source.size(), copy.size());
 }
+
+TEST(PickleTest, EvilLengths) {
+  Pickle source;
+  std::string str(10000, 'A');
+  source.WriteData(str.c_str(), 100000);
+  // ReadString16 used to have its read buffer length calculation wrong leading
+  // to out-of-bounds reading.
+  void* iter = NULL;
+  string16 str16;
+  EXPECT_FALSE(source.ReadString16(&iter, &str16));
+
+  // And check we didn't break ReadString16.
+  str16 = (wchar_t) 'A';
+  Pickle str16_pickle;
+  str16_pickle.WriteString16(str16);
+  iter = NULL;
+  EXPECT_TRUE(str16_pickle.ReadString16(&iter, &str16));
+  EXPECT_EQ(1U, str16.length());
+
+  // Check we don't fail in a length check with large WStrings.
+  Pickle big_len;
+  big_len.WriteInt(1 << 30);
+  iter = NULL;
+  std::wstring wstr;
+  EXPECT_FALSE(big_len.ReadWString(&iter, &wstr));
+}
+