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

components/sync/syncable/mutable_entry.cc

 // Copyright 2012 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/mutable_entry.h"
 
-#include <memory>
+#include <utility>
 
 #include "components/sync/base/hash_util.h"
 #include "components/sync/base/unique_position.h"
 #include "components/sync/syncable/directory.h"
 #include "components/sync/syncable/scoped_kernel_lock.h"
 #include "components/sync/syncable/scoped_parent_child_index_updater.h"
 #include "components/sync/syncable/syncable_changes_version.h"
 #include "components/sync/syncable/syncable_write_transaction.h"
 
 using std::string;
 
 namespace syncer {
 namespace syncable {
 
-void MutableEntry::Init(WriteTransaction* trans,
-                        ModelType model_type,
-                        const Id& parent_id,
-                        const string& name) {
-  std::unique_ptr<EntryKernel> kernel(new EntryKernel);
-  kernel_ = nullptr;
-
-  kernel->put(ID, trans->directory_->NextId());
-  kernel->put(META_HANDLE, trans->directory_->NextMetahandle());
-  kernel->mark_dirty(&trans->directory_->kernel()->dirty_metahandles);
-  kernel->put(NON_UNIQUE_NAME, name);
-  const base::Time& now = base::Time::Now();
-  kernel->put(CTIME, now);
-  kernel->put(MTIME, now);
-  // We match the database defaults here
-  kernel->put(BASE_VERSION, CHANGES_VERSION);
-
-  if (!parent_id.IsNull()) {
-    kernel->put(PARENT_ID, parent_id);
-  }
-
-  // Normally the SPECIFICS setting code is wrapped in logic to deal with
-  // unknown fields and encryption.  Since all we want to do here is ensure that
-  // GetModelType() returns a correct value from the very beginning, these
-  // few lines are sufficient.
-  sync_pb::EntitySpecifics specifics;
-  AddDefaultFieldValue(model_type, &specifics);
-  kernel->put(SPECIFICS, specifics);
-
-  // Because this entry is new, it was originally deleted.
-  kernel->put(IS_DEL, true);
-  trans->TrackChangesTo(kernel.get());
-  kernel->put(IS_DEL, false);
-
-  // Now swap the pointers.
-  kernel_ = kernel.release();
-}
-
 MutableEntry::MutableEntry(WriteTransaction* trans,
                            Create,
                            ModelType model_type,
                            const string& name)
     : ModelNeutralMutableEntry(trans), write_transaction_(trans) {
-  Init(trans, model_type, Id(), name);
+  std::unique_ptr<EntryKernel> kernel =
+      CreateEntryKernel(trans, model_type, Id(), name);
+  kernel_ = kernel.get();
   // We need to have a valid position ready before we can index the item.
   DCHECK_NE(BOOKMARKS, model_type);
   DCHECK(!ShouldMaintainPosition());
 
-  bool result = trans->directory()->InsertEntry(trans, kernel_);
+  bool result = trans->directory()->InsertEntry(trans, std::move(kernel));
   DCHECK(result);
 }
 
 MutableEntry::MutableEntry(WriteTransaction* trans,
                            Create,
                            ModelType model_type,
                            const Id& parent_id,
                            const string& name)
     : ModelNeutralMutableEntry(trans), write_transaction_(trans) {
-  Init(trans, model_type, parent_id, name);
+  std::unique_ptr<EntryKernel> kernel =
+      CreateEntryKernel(trans, model_type, parent_id, name);
+  kernel_ = kernel.get();
   // We need to have a valid position ready before we can index the item.
   if (model_type == BOOKMARKS) {
     // Base the tag off of our cache-guid and local "c-" style ID.
     std::string unique_tag = GenerateSyncableBookmarkHash(
         trans->directory()->cache_guid(), GetId().GetServerId());
     kernel_->put(UNIQUE_BOOKMARK_TAG, unique_tag);
     kernel_->put(UNIQUE_POSITION, UniquePosition::InitialPosition(unique_tag));
   } else {
     DCHECK(!ShouldMaintainPosition());
   }
 
-  bool result = trans->directory()->InsertEntry(trans, kernel_);
+  bool result = trans->directory()->InsertEntry(trans, std::move(kernel));
   DCHECK(result);
 }
 
 MutableEntry::MutableEntry(WriteTransaction* trans,
                            CreateNewUpdateItem,
                            const Id& id)
     : ModelNeutralMutableEntry(trans, CREATE_NEW_UPDATE_ITEM, id),
       write_transaction_(trans) {}
 
 MutableEntry::MutableEntry(WriteTransaction* trans, GetById, const Id& id)
@@ skipping 193 matching lines @@
         attachment_metadata.mutable_record(i);
     if (record->id().unique_id() != attachment_id.unique_id())
       continue;
     record->set_is_on_server(true);
   }
   kernel_->put(ATTACHMENT_METADATA, attachment_metadata);
   MarkDirty();
   MarkForSyncing(this);
 }
 
+// static
+std::unique_ptr<EntryKernel> MutableEntry::CreateEntryKernel(
+    WriteTransaction* trans,
+    ModelType model_type,
+    const Id& parent_id,
+    const string& name) {
+  std::unique_ptr<EntryKernel> kernel(new EntryKernel);
+
+  kernel->put(ID, trans->directory_->NextId());
+  kernel->put(META_HANDLE, trans->directory_->NextMetahandle());
+  kernel->mark_dirty(&trans->directory_->kernel()->dirty_metahandles);
+  kernel->put(NON_UNIQUE_NAME, name);
+  const base::Time& now = base::Time::Now();
+  kernel->put(CTIME, now);
+  kernel->put(MTIME, now);
+  // We match the database defaults here
+  kernel->put(BASE_VERSION, CHANGES_VERSION);
+
+  if (!parent_id.IsNull()) {
+    kernel->put(PARENT_ID, parent_id);
+  }
+
+  // Normally the SPECIFICS setting code is wrapped in logic to deal with
+  // unknown fields and encryption.  Since all we want to do here is ensure that
+  // GetModelType() returns a correct value from the very beginning, these
+  // few lines are sufficient.
+  sync_pb::EntitySpecifics specifics;
+  AddDefaultFieldValue(model_type, &specifics);
+  kernel->put(SPECIFICS, specifics);
+
+  // Because this entry is new, it was originally deleted.
+  kernel->put(IS_DEL, true);
+  trans->TrackChangesTo(kernel.get());
+  kernel->put(IS_DEL, false);
+  return kernel;
+}
+
 // This function sets only the flags needed to get this entry to sync.
 bool MarkForSyncing(MutableEntry* e) {
   DCHECK_NE(static_cast<MutableEntry*>(nullptr), e);
   DCHECK(!e->IsRoot()) << "We shouldn't mark a permanent object for syncing.";
   if (!(e->PutIsUnsynced(true)))
     return false;
   if (e->GetSyncing())
     e->PutDirtySync(true);
   return true;
 }
 
 }  // namespace syncable
 }  // namespace syncer