fix: use a WeakPtr so we do not UAF the store in FunctionLifetimeMoni…

…tor (#22112) Co-authored-by: Samuel Attard <samuel.r.attard@gmail.com>
electron · Feb 10, 2020 · a305444 · a305444
1 parent 1f631ca
commit a305444
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 11 deletions.
diff --git a/shell/renderer/api/atom_api_context_bridge.cc b/shell/renderer/api/atom_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);
     }

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
@@ -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_;