Avoid calling vector resize() with excessive size parameter: fix broken integer overflow checks, or remove resize() calls to simplify non-hot-path cases, or add stronger validations as appropriate.

BUG=31364
TEST=NONE

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

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

3 files changed, 23 insertions, 21 deletions

diff --git a/base/histogram.cc b/base/histogram.cc
index b7f6cb2..03478d7 100644
--- a/base/histogram.cc
+++ b/base/histogram.cc
@@ -193,7 +193,6 @@ void Histogram::Initialize() {
     declared_min_ = 1;
   if (declared_max_ >= kSampleType_MAX)
     declared_max_ = kSampleType_MAX - 1;
-  DCHECK_GT(declared_min_, 0);  // We provide underflow bucket.
   DCHECK(declared_min_ <= declared_max_);
   DCHECK_LT(1u, bucket_count_);
   size_t maximal_bucket_count = declared_max_ - declared_min_ + 2;
@@ -423,9 +422,17 @@ bool Histogram::DeserializeHistogramInfo(const std::string& histogram_info) {
       !pickle.ReadInt(&iter, &histogram_type) ||
       !pickle.ReadInt(&iter, &pickle_flags) ||
       !sample.Histogram::SampleSet::Deserialize(&iter, pickle)) {
-    LOG(ERROR) << "Picke error decoding Histogram: " << histogram_name;
+    LOG(ERROR) << "Pickle error decoding Histogram: " << histogram_name;
     return false;
   }
+  // Since these fields may have come from an untrusted renderer, do additional
+  // checks above and beyond those in Histogram::Initialize()
+  if (declared_max <= 0 || declared_min <= 0 || declared_max < declared_min ||
+      INT_MAX / sizeof(Count) <= bucket_count || bucket_count < 2) {
+    LOG(ERROR) << "Values error decoding Histogram: " << histogram_name;
+    return false;
+  }
+
   Flags flags = static_cast<Flags>(pickle_flags & ~kIPCSerializationSourceFlag);
 
   DCHECK(histogram_type != NOT_VALID_IN_RENDERER);
@@ -548,14 +555,14 @@ bool Histogram::SampleSet::Deserialize(void** iter, const Pickle& pickle) {
     return false;
   }
 
-  if (counts_size <= 0)
+  if (counts_size == 0)
     return false;
 
-  counts_.resize(counts_size, 0);
   for (size_t index = 0; index < counts_size; ++index) {
-    if (!pickle.ReadInt(iter, &counts_[index])) {
+    int i;
+    if (!pickle.ReadInt(iter, &i))
       return false;
-    }
+    counts_.push_back(i);
   }
 
   return true;
diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h
index 555ebd0..e21c415 100644
--- a/chrome/common/render_messages.h
+++ b/chrome/common/render_messages.h
@@ -861,14 +861,15 @@ struct ParamTraits<webkit_glue::FormFieldValues> {
           ReadParam(m, iter, &p->target_url);
       size_t elements_size = 0;
       result = result && ReadParam(m, iter, &elements_size);
-      p->elements.resize(elements_size);
       for (size_t i = 0; i < elements_size; i++) {
         string16 label, name, type, value;
         result = result && ReadParam(m, iter, &label);
         result = result && ReadParam(m, iter, &name);
         result = result && ReadParam(m, iter, &type);
         result = result && ReadParam(m, iter, &value);
-        p->elements[i] = webkit_glue::FormField(label, name, type, value);
+        if (result)
+          p->elements.push_back(
+            webkit_glue::FormField(label, name, type, value));
       }
       return result;
   }
diff --git a/ipc/ipc_message_utils.h b/ipc/ipc_message_utils.h
index 76811a7..4f1a6f32 100644
--- a/ipc/ipc_message_utils.h
+++ b/ipc/ipc_message_utils.h
@@ -451,22 +451,16 @@ struct ParamTraits<std::vector<P> > {
   }
   static bool Read(const Message* m, void** iter, param_type* r) {
     int size;
+    // ReadLength() checks for < 0 itself.
     if (!m->ReadLength(iter, &size))
       return false;
     // Resizing beforehand is not safe, see BUG 1006367 for details.
-    if (m->IteratorHasRoomFor(*iter, size * sizeof(P))) {
-      r->resize(size);
-      for (int i = 0; i < size; i++) {
-        if (!ReadParam(m, iter, &(*r)[i]))
-          return false;
-      }
-    } else {
-      for (int i = 0; i < size; i++) {
-        P element;
-        if (!ReadParam(m, iter, &element))
-          return false;
-        r->push_back(element);
-      }
+    if (INT_MAX / sizeof(P) <= static_cast<size_t>(size))
+      return false;
+    r->resize(size);
+    for (int i = 0; i < size; i++) {
+      if (!ReadParam(m, iter, &(*r)[i]))
+        return false;
     }
     return true;
   }