From 4e6d985088b552b0d0acbb36c36dd0992784c75f Mon Sep 17 00:00:00 2001
From: Samuel Attard <sattard@slack-corp.com>
Date: Wed, 5 Feb 2020 15:00:06 -0800
Subject: [PATCH] fix: use a WeakPtr so we do not UAF the store in
 FunctionLifetimeMonitor

---
 .../render_frame_context_bridge_store.h       |  4 +++
 .../api/electron_api_context_bridge.cc        | 28 +++++++++++--------
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/shell/renderer/api/context_bridge/render_frame_context_bridge_store.h b/shell/renderer/api/context_bridge/render_frame_context_bridge_store.h
index 2d0a6c0f4f7c1..3321785d59bbe 100644
--- a/shell/renderer/api/context_bridge/render_frame_context_bridge_store.h
+++ b/shell/renderer/api/context_bridge/render_frame_context_bridge_store.h
@@ -50,6 +50,10 @@ class RenderFramePersistenceStore final : public content::RenderFrameObserver {
                           v8::Local<v8::Value> proxy_value);
   v8::MaybeLocal<v8::Value> GetCachedProxiedObject(v8::Local<v8::Value> from);
 
+  base::WeakPtr<RenderFramePersistenceStore> GetWeakPtr() {
+    return weak_factory_.GetWeakPtr();
+  }
+
  private:
   // func_id ==> { function, owning_context }
   std::map<size_t, FunctionContextPair> functions_;
diff --git a/shell/renderer/api/electron_api_context_bridge.cc b/shell/renderer/api/electron_api_context_bridge.cc
index 216436a7c5154..2217417cb48ea 100644
--- a/shell/renderer/api/electron_api_context_bridge.cc
+++ b/shell/renderer/api/electron_api_context_bridge.cc
@@ -98,25 +98,31 @@ bool IsPlainArray(const v8::Local<v8::Value>& arr) {
 
 class FunctionLifeMonitor final : public ObjectLifeMonitor {
  public:
-  static void BindTo(v8::Isolate* isolate,
-                     v8::Local<v8::Object> target,
-                     context_bridge::RenderFramePersistenceStore* store,
-                     size_t func_id) {
+  static void BindTo(
+      v8::Isolate* isolate,
+      v8::Local<v8::Object> target,
+      base::WeakPtr<context_bridge::RenderFramePersistenceStore> store,
+      size_t func_id) {
     new FunctionLifeMonitor(isolate, target, store, func_id);
   }
 
  protected:
-  FunctionLifeMonitor(v8::Isolate* isolate,
-                      v8::Local<v8::Object> target,
-                      context_bridge::RenderFramePersistenceStore* store,
-                      size_t func_id)
+  FunctionLifeMonitor(
+      v8::Isolate* isolate,
+      v8::Local<v8::Object> target,
+      base::WeakPtr<context_bridge::RenderFramePersistenceStore> store,
+      size_t func_id)
       : ObjectLifeMonitor(isolate, target), store_(store), func_id_(func_id) {}
   ~FunctionLifeMonitor() override = default;
 
-  void RunDestructor() override { store_->functions().erase(func_id_); }
+  void RunDestructor() override {
+    if (!store_)
+      return;
+    store_->functions().erase(func_id_);
+  }
 
  private:
-  context_bridge::RenderFramePersistenceStore* store_;
+  base::WeakPtr<context_bridge::RenderFramePersistenceStore> store_;
   size_t func_id_;
 };
 
@@ -173,7 +179,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
           base::BindRepeating(&ProxyFunctionWrapper, store, func_id));
       FunctionLifeMonitor::BindTo(destination_context->GetIsolate(),
                                   v8::Local<v8::Object>::Cast(proxy_func),
-                                  store, func_id);
+                                  store->GetWeakPtr(), func_id);
       store->CacheProxiedObject(value, proxy_func);
       return v8::MaybeLocal<v8::Value>(proxy_func);
     }