Remove vector preallocation in BookmarkNodeData::Element::ReadFromPickle

Preallocation in BookmarkNodeData::Element::ReadFromPickle can be
abused by a pickle consisting of N nested children. If each child
claims to contain M more children, the fast-fail logic (returns false
when fewer than M children were provided) would never run until
reading the last element at depth N. Along the way, we would have
allocated N * M elements rather than N.

Bug: 966282
Change-Id: I02de64d091091e5ab5b1628cabe9a5caea6fbe2c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1628836Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Dan McArdle <dmcardle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#663988}

components/bookmarks/browser/bookmark_node_data.cc

@@ -97,13 +97,14 @@ bool BookmarkNodeData::Element::ReadFromPickle(base::PickleIterator* iterator) {
       LOG(WARNING) << "children_count failed bounds check";
       return false;
     }
+    // Note: do not preallocate the children vector. A pickle could be
+    // constructed to contain N nested Elements. By continually recursing on
+    // this ReadFromPickle function, the fast-fail logic is subverted. Each
+    // child would claim it contains M more children. The first (and only) child
+    // provided would claim M more children. We would allocate N * M Elements
+    // along the way, while only receiving N Elements.
     const size_t children_count =
         base::checked_cast<size_t>(children_count_tmp);
-    // Restrict vector preallocation to prevent OOM crashes on invalid (or
-    // malicious) pickles.
-    if (children_count > kMaxVectorPreallocateSize)
-      LOG(WARNING) << "children_count exceeds kMaxVectorPreallocateSize";
-    children.reserve(std::min(children_count, kMaxVectorPreallocateSize));
     for (size_t i = 0; i < children_count; ++i) {
       children.emplace_back();
       if (!children.back().ReadFromPickle(iterator))