[Backport] Security issue 867306

Keep reference to DOMStorageNamespace while it's being cloned

While DOMStorageNamespace::Clone constructs an instance, it binds it to
a callback, post it to a task runner and returns the instance as a raw
pointer. Note that base::BindOnce here retains a reference to |clone|
and releases the reference when the callback instance is destroyed.

However, if PostTaskAndReply there failed, the callback instance is
destroyed immediately and DOMStorageNamespace loses the last reference.
Then, DOMStorageNamespace::Clone may return a stale pointer.

This CL converts the return value to scoped_refptr, and has Clone() to
keep the reference to the resulting instance.

Bug: 866456, 867306
Change-Id: I54a330b2905c0d697ee31c3ab95764ecbb72abe1
Reviewed-on: https://chromium-review.googlesource.com/1146409
Reviewed-on: https://chromium-review.googlesource.com/1152588
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

2 files changed, 5 insertions, 4 deletions

diff --git a/chromium/content/browser/dom_storage/dom_storage_namespace.cc b/chromium/content/browser/dom_storage/dom_storage_namespace.cc
index 2447ac8350b..f8894ca665f 100644
--- a/chromium/content/browser/dom_storage/dom_storage_namespace.cc
+++ b/chromium/content/browser/dom_storage/dom_storage_namespace.cc
@@ -76,12 +76,12 @@ DOMStorageArea* DOMStorageNamespace::GetOpenStorageArea(const GURL& origin) {
   return nullptr;
 }
 
-DOMStorageNamespace* DOMStorageNamespace::Clone(
+scoped_refptr<DOMStorageNamespace> DOMStorageNamespace::Clone(
     int64_t clone_namespace_id,
     const std::string& clone_persistent_namespace_id) {
   DCHECK_NE(kLocalStorageNamespaceId, namespace_id_);
   DCHECK_NE(kLocalStorageNamespaceId, clone_namespace_id);
-  DOMStorageNamespace* clone = new DOMStorageNamespace(
+  auto clone = base::MakeRefCounted<DOMStorageNamespace>(
       clone_namespace_id, clone_persistent_namespace_id,
       session_storage_database_.get(), task_runner_.get());
   AreaMap::const_iterator it = areas_.begin();
diff --git a/chromium/content/browser/dom_storage/dom_storage_namespace.h b/chromium/content/browser/dom_storage/dom_storage_namespace.h
index 9eb8bb59620..693758f4338 100644
--- a/chromium/content/browser/dom_storage/dom_storage_namespace.h
+++ b/chromium/content/browser/dom_storage/dom_storage_namespace.h
@@ -68,8 +68,9 @@ class CONTENT_EXPORT DOMStorageNamespace
   // Creates a clone of |this| namespace including
   // shallow copies of all contained areas.
   // Should only be called for session storage namespaces.
-  DOMStorageNamespace* Clone(int64_t clone_namespace_id,
-                             const std::string& clone_persistent_namespace_id);
+  scoped_refptr<DOMStorageNamespace> Clone(
+      int64_t clone_namespace_id,
+      const std::string& clone_persistent_namespace_id);
 
   void DeleteLocalStorageOrigin(const GURL& origin);
   void DeleteSessionStorageOrigin(const GURL& origin);