[Sync] Address use-after-free in Directory::InsertEntry

components/sync/syncable/directory.cc

 // Copyright 2013 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 "components/sync/syncable/directory.h"
 
 #include <inttypes.h>
 
 #include <algorithm>
 #include <iterator>
@@ skipping 336 matching lines @@
 
 int Directory::GetPositionIndex(BaseTransaction* trans,
                                 EntryKernel* kernel) const {
   const OrderedChildSet* siblings =
       kernel_->parent_child_index.GetSiblings(kernel);
 
   OrderedChildSet::const_iterator it = siblings->find(kernel);
   return std::distance(siblings->begin(), it);
 }
 
-bool Directory::InsertEntry(BaseWriteTransaction* trans, EntryKernel* entry) {
+bool Directory::InsertEntry(BaseWriteTransaction* trans,
+                            std::unique_ptr<EntryKernel> entry) {
   ScopedKernelLock lock(this);
-  return InsertEntry(lock, trans, entry);
+  return InsertEntry(lock, trans, std::move(entry));
 }
 
 bool Directory::InsertEntry(const ScopedKernelLock& lock,
                             BaseWriteTransaction* trans,
-                            EntryKernel* entry) {
+                            std::unique_ptr<EntryKernel> entry) {
   if (!SyncAssert(nullptr != entry, FROM_HERE, "Entry is null", trans))
     return false;
+  EntryKernel* entry_ptr = entry.get();
 
   static const char error[] = "Entry already in memory index.";
 
   if (!SyncAssert(kernel_->metahandles_map
-                      .insert(std::make_pair(entry->ref(META_HANDLE),
-                                             base::WrapUnique(entry)))
+                      .insert(std::make_pair(entry_ptr->ref(META_HANDLE),
+                                             std::move(entry)))
                       .second,
                   FROM_HERE, error, trans)) {
     return false;
   }
   if (!SyncAssert(
-          kernel_->ids_map.insert(std::make_pair(entry->ref(ID).value(), entry))
+          kernel_->ids_map
+              .insert(std::make_pair(entry_ptr->ref(ID).value(), entry_ptr))
               .second,
           FROM_HERE, error, trans)) {
     return false;
   }
-  if (ParentChildIndex::ShouldInclude(entry)) {
-    if (!SyncAssert(kernel_->parent_child_index.Insert(entry), FROM_HERE, error,
-                    trans)) {
+  if (ParentChildIndex::ShouldInclude(entry_ptr)) {
+    if (!SyncAssert(kernel_->parent_child_index.Insert(entry_ptr), FROM_HERE,
+                    error, trans)) {
       return false;
     }
   }
-  AddToAttachmentIndex(lock, entry->ref(META_HANDLE),
-                       entry->ref(ATTACHMENT_METADATA));
+  AddToAttachmentIndex(lock, entry_ptr->ref(META_HANDLE),
+                       entry_ptr->ref(ATTACHMENT_METADATA));
 
   // Should NEVER be created with a client tag or server tag.
-  if (!SyncAssert(entry->ref(UNIQUE_SERVER_TAG).empty(), FROM_HERE,
+  if (!SyncAssert(entry_ptr->ref(UNIQUE_SERVER_TAG).empty(), FROM_HERE,
                   "Server tag should be empty", trans)) {
     return false;
   }
-  if (!SyncAssert(entry->ref(UNIQUE_CLIENT_TAG).empty(), FROM_HERE,
+  if (!SyncAssert(entry_ptr->ref(UNIQUE_CLIENT_TAG).empty(), FROM_HERE,
                   "Client tag should be empty", trans))
     return false;
 
   return true;
 }
 
 bool Directory::ReindexId(BaseWriteTransaction* trans,
                           EntryKernel* const entry,
                           const Id& new_id) {
   ScopedKernelLock lock(this);
@@ skipping 202 matching lines @@
     return true;
 
   // Need a write transaction as we are about to permanently purge entries.
   WriteTransaction trans(FROM_HERE, VACUUM_AFTER_SAVE, this);
   ScopedKernelLock lock(this);
   // Now drop everything we can out of memory.
   for (auto i = snapshot.dirty_metas.begin(); i != snapshot.dirty_metas.end();
        ++i) {
     MetahandlesMap::iterator found =
         kernel_->metahandles_map.find((*i)->ref(META_HANDLE));
-    EntryKernel* entry =
-        (found == kernel_->metahandles_map.end() ? nullptr
-                                                 : found->second.get());
-    if (entry && SafeToPurgeFromMemory(&trans, entry)) {
+    if (found != kernel_->metahandles_map.end() &&
+        SafeToPurgeFromMemory(&trans, found->second.get())) {
       // We now drop deleted metahandles that are up to date on both the client
       // and the server.
-      std::unique_ptr<EntryKernel> unique_entry = std::move(found->second);
+      std::unique_ptr<EntryKernel> entry = std::move(found->second);
 
       size_t num_erased = 0;
-      num_erased = kernel_->metahandles_map.erase(entry->ref(META_HANDLE));
-      DCHECK_EQ(1u, num_erased);
+      kernel_->metahandles_map.erase(found);
       num_erased = kernel_->ids_map.erase(entry->ref(ID).value());
       DCHECK_EQ(1u, num_erased);
       if (!entry->ref(UNIQUE_SERVER_TAG).empty()) {
         num_erased =
             kernel_->server_tags_map.erase(entry->ref(UNIQUE_SERVER_TAG));
         DCHECK_EQ(1u, num_erased);
       }
       if (!entry->ref(UNIQUE_CLIENT_TAG).empty()) {
         num_erased =
             kernel_->client_tags_map.erase(entry->ref(UNIQUE_CLIENT_TAG));
         DCHECK_EQ(1u, num_erased);
       }
-      if (!SyncAssert(!kernel_->parent_child_index.Contains(entry), FROM_HERE,
-                      "Deleted entry still present", (&trans)))
+      if (!SyncAssert(!kernel_->parent_child_index.Contains(entry.get()),
+                      FROM_HERE, "Deleted entry still present", (&trans)))
         return false;
       RemoveFromAttachmentIndex(lock, entry->ref(META_HANDLE),
                                 entry->ref(ATTACHMENT_METADATA));
     }
     if (trans.unrecoverable_error_set())
       return false;
   }
   return true;
 }
 
@@ skipping 48 matching lines @@
   }
 
   // At this point locally created items that aren't synced will become locally
   // deleted items, and purged on the next snapshot. All other items will match
   // the state they would have had if they were just created via a server
   // update. See MutableEntry::MutableEntry(.., CreateNewUpdateItem, ..).
 }
 
 void Directory::DeleteEntry(const ScopedKernelLock& lock,
                             bool save_to_journal,
-                            EntryKernel* entry,
+                            EntryKernel* entry_ptr,
                             OwnedEntryKernelSet* entries_to_journal) {
-  int64_t handle = entry->ref(META_HANDLE);
-  ModelType server_type =
-      GetModelTypeFromSpecifics(entry->ref(SERVER_SPECIFICS));
+  int64_t handle = entry_ptr->ref(META_HANDLE);
 
   kernel_->metahandles_to_purge.insert(handle);
 
-  std::unique_ptr<EntryKernel> entry_ptr =
+  std::unique_ptr<EntryKernel> entry =
       std::move(kernel_->metahandles_map[handle]);
 
+  ModelType server_type =
+      GetModelTypeFromSpecifics(entry->ref(SERVER_SPECIFICS));
+
   size_t num_erased = 0;
   num_erased = kernel_->metahandles_map.erase(handle);
   DCHECK_EQ(1u, num_erased);
   num_erased = kernel_->ids_map.erase(entry->ref(ID).value());
   DCHECK_EQ(1u, num_erased);
   num_erased = kernel_->unsynced_metahandles.erase(handle);
   DCHECK_EQ(entry->ref(IS_UNSYNCED), num_erased > 0);
   num_erased = kernel_->unapplied_update_metahandles[server_type].erase(handle);
   DCHECK_EQ(entry->ref(IS_UNAPPLIED_UPDATE), num_erased > 0);
-  if (kernel_->parent_child_index.Contains(entry))
-    kernel_->parent_child_index.Remove(entry);
+  if (kernel_->parent_child_index.Contains(entry.get()))
+    kernel_->parent_child_index.Remove(entry.get());
 
   if (!entry->ref(UNIQUE_CLIENT_TAG).empty()) {
     num_erased = kernel_->client_tags_map.erase(entry->ref(UNIQUE_CLIENT_TAG));
     DCHECK_EQ(1u, num_erased);
   }
   if (!entry->ref(UNIQUE_SERVER_TAG).empty()) {
     num_erased = kernel_->server_tags_map.erase(entry->ref(UNIQUE_SERVER_TAG));
     DCHECK_EQ(1u, num_erased);
   }
   RemoveFromAttachmentIndex(lock, handle, entry->ref(ATTACHMENT_METADATA));
 
   if (save_to_journal) {
-    entries_to_journal->insert(std::move(entry_ptr));
+    entries_to_journal->insert(std::move(entry));
   }
 }
 
 void Directory::PurgeEntriesWithTypeIn(ModelTypeSet disabled_types,
                                        ModelTypeSet types_to_journal,
                                        ModelTypeSet types_to_unapply) {
   disabled_types.RemoveAll(ProxyTypes());
   if (disabled_types.Empty())
     return;
 
@@ skipping 875 matching lines @@
 Directory::Kernel* Directory::kernel() {
   return kernel_;
 }
 
 const Directory::Kernel* Directory::kernel() const {
   return kernel_;
 }
 
 }  // namespace syncable
 }  // namespace syncer