Browse Source

fix: heap-use-after-free in tray.popUpContextMenu (#22842) (#23182)

Jeremy Apthorp 5 years ago
parent
commit
fb6f60460b
2 changed files with 15 additions and 0 deletions
  1. 8 0
      shell/browser/ui/tray_icon_cocoa.mm
  2. 7 0
      spec-main/api-tray-spec.ts

+ 8 - 0
shell/browser/ui/tray_icon_cocoa.mm

@@ -173,8 +173,16 @@
                             useDefaultAccelerator:NO]);
     // Hacky way to mimic design of ordinary tray menu.
     [statusItem_ setMenu:[menuController menu]];
+    // -performClick: is a blocking call, which will run the task loop inside
+    // itself. This can potentially include running JS, which can result in
+    // this object being released. We take a temporary reference here to make
+    // sure we stay alive long enough to successfully return from this
+    // function.
+    // TODO(nornagon/codebytere): Avoid nesting task loops here.
+    [self retain];
     [[statusItem_ button] performClick:self];
     [statusItem_ setMenu:[menuController_ menu]];
+    [self release];
     return;
   }
 

+ 7 - 0
spec-main/api-tray-spec.ts

@@ -51,6 +51,13 @@ describe('tray module', () => {
       })
       tray.popUpContextMenu()
     })
+
+    it('can be called with a menu', () => {
+      const menu = Menu.buildFromTemplate([{ label: 'Test' }])
+      expect(() => {
+        tray.popUpContextMenu(menu)
+      }).to.not.throw()
+    })
   })
 
   describe('tray.getBounds()', () => {