From 6ab51a4e55cffc1cf23db9397bba5c9e8a40378b Mon Sep 17 00:00:00 2001
From: Marshall Greenblatt <magreenblatt@gmail.com>
Date: Fri, 29 Apr 2016 14:09:35 -0700
Subject: [PATCH] Fix heap-use-after-free during CefCookieManagerImpl
 destruction (issue #1882)

---
 libcef/browser/cookie_manager_impl.cc | 20 ++++++++++++++++++--
 libcef/browser/cookie_manager_impl.h  |  4 ++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/libcef/browser/cookie_manager_impl.cc b/libcef/browser/cookie_manager_impl.cc
index c4a31cc84..c4b0aad2e 100644
--- a/libcef/browser/cookie_manager_impl.cc
+++ b/libcef/browser/cookie_manager_impl.cc
@@ -113,10 +113,18 @@ void SetCookieCallbackImpl(CefRefPtr<CefSetCookieCallback> callback,
       base::Bind(&CefSetCookieCallback::OnComplete, callback.get(), success));
 }
 
+net::CookieStore* GetExistingCookieStoreHelper(
+    base::WeakPtr<CefCookieManagerImpl> cookie_manager) {
+  if (cookie_manager.get())
+    return cookie_manager->GetExistingCookieStore();
+  return nullptr;
+}
+
 }  // namespace
 
 
-CefCookieManagerImpl::CefCookieManagerImpl() {
+CefCookieManagerImpl::CefCookieManagerImpl()
+    : weak_ptr_factory_(this) {
 }
 
 CefCookieManagerImpl::~CefCookieManagerImpl() {
@@ -160,8 +168,16 @@ void CefCookieManagerImpl::GetCookieStore(
 
   DCHECK(cookie_store_.get());
 
+  // Binding ref-counted |this| to CookieStoreGetter may result in
+  // heap-use-after-free if (a) the CookieStoreGetter contains the last
+  // CefCookieManagerImpl reference and (b) that reference is released during
+  // execution of a CookieMonster callback (which then results in the
+  // CookieManager being deleted). Use WeakPtr instead of |this| so that, in
+  // that case, the CookieStoreGetter will return nullptr instead of keeping
+  // the CefCookieManagerImpl alive (see issue #1882).
   const CookieStoreGetter& cookie_store_getter =
-      base::Bind(&CefCookieManagerImpl::GetExistingCookieStore, this);
+      base::Bind(GetExistingCookieStoreHelper,
+                 weak_ptr_factory_.GetWeakPtr());
 
   if (task_runner->BelongsToCurrentThread()) {
     // Execute the callback immediately.
diff --git a/libcef/browser/cookie_manager_impl.h b/libcef/browser/cookie_manager_impl.h
index 660cf3adf..773cb68ab 100644
--- a/libcef/browser/cookie_manager_impl.h
+++ b/libcef/browser/cookie_manager_impl.h
@@ -12,6 +12,7 @@
 #include "libcef/browser/thread_util.h"
 
 #include "base/files/file_path.h"
+#include "base/memory/weak_ptr.h"
 #include "net/cookies/cookie_monster.h"
 
 // Implementation of the CefCookieManager interface.
@@ -126,6 +127,9 @@ class CefCookieManagerImpl : public CefCookieManager {
   std::vector<std::string> supported_schemes_;
   std::unique_ptr<net::CookieMonster> cookie_store_;
 
+  // Must be the last member.
+  base::WeakPtrFactory<CefCookieManagerImpl> weak_ptr_factory_;
+
   IMPLEMENT_REFCOUNTING_DELETE_ON_IOT(CefCookieManagerImpl);
 };