Do permission check when calling guest window methods

atom/common/api/atom_api_v8_util.cc

@@ -9,9 +9,11 @@
 #include "atom/common/api/remote_callback_freer.h"
 #include "atom/common/api/remote_object_freer.h"
 #include "atom/common/native_mate_converters/content_converter.h"
+#include "atom/common/native_mate_converters/gurl_converter.h"
 #include "atom/common/node_includes.h"
 #include "base/hash.h"
 #include "native_mate/dictionary.h"
+#include "url/origin.h"
 #include "v8/include/v8-profiler.h"
 
 namespace std {
@@ -97,6 +99,10 @@ void RequestGarbageCollectionForTesting(v8::Isolate* isolate) {
     v8::Isolate::GarbageCollectionType::kFullGarbageCollection);
 }
 
+bool IsSameOrigin(const GURL& l, const GURL& r) {
+  return url::Origin(l).IsSameOriginWith(url::Origin(r));
+}
+
 void Initialize(v8::Local<v8::Object> exports, v8::Local<v8::Value> unused,
                 v8::Local<v8::Context> context, void* priv) {
   mate::Dictionary dict(context->GetIsolate(), exports);
@@ -112,6 +118,7 @@ void Initialize(v8::Local<v8::Object> exports, v8::Local<v8::Value> unused,
                  &atom::api::KeyWeakMap<std::pair<int64_t, int32_t>>::Create);
   dict.SetMethod("requestGarbageCollectionForTesting",
                  &RequestGarbageCollectionForTesting);
+  dict.SetMethod("isSameOrigin", &IsSameOrigin);
 }
 
 }  // namespace

lib/browser/guest-window-manager.js

@@ -1,6 +1,7 @@
 'use strict'
 
 const {BrowserWindow, ipcMain, webContents} = require('electron')
+const {isSameOrigin} = process.atomBinding('v8_util')
 
 const hasProp = {}.hasOwnProperty
 const frameToGuest = {}
@@ -151,6 +152,22 @@ const getGuestWindow = function (guestId) {
   return guestWindow
 }
 
+// Checks whether |sender| can access the |target|:
+// 1. Check whether |sender| is the parent of |target|.
+// 2. Check whether |sender| has node integration, if so it is allowed to
+//    do anything it wants.
+// 3. Check whether the origins match.
+//
+// However it allows a child window without node integration but with same
+// origin to do anything it wants, when its opener window has node integration.
+// The W3C does not have anything on this, but from my understanding of the
+// security model of |window.opener|, this should be fine.
+const canAccessWindow = function (sender, target) {
+  return (target.getWebPreferences().openerId === sender.webContents.id) ||
+         (sender.getWebPreferences().nodeIntegration === true) ||
+         isSameOrigin(sender.getURL(), target.getURL())
+}
+
 // Routed window.open messages.
 ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_OPEN', function (event, url, frameName,
                                                                   disposition, options,
@@ -171,18 +188,27 @@ ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_OPEN', function (event, url, fr
 
 ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_CLOSE', function (event, guestId) {
   const guestWindow = getGuestWindow(guestId)
-  if (guestWindow != null) guestWindow.destroy()
+  if (guestWindow != null && canAccessWindow(event.sender, guestWindow.webContents)) {
+    guestWindow.destroy()
+  }
 })
 
 ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_METHOD', function (event, guestId, method, ...args) {
   const guestWindow = getGuestWindow(guestId)
-  event.returnValue = guestWindow != null ? guestWindow[method].apply(guestWindow, args) : null
+  if (guestWindow != null && canAccessWindow(event.sender, guestWindow.webContents)) {
+    event.returnValue = guestWindow[method].apply(guestWindow, args)
+  } else {
+    event.returnValue = null
+  }
 })
 
 ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_POSTMESSAGE', function (event, guestId, message, targetOrigin, sourceOrigin) {
   const guestContents = webContents.fromId(guestId)
   if (guestContents == null) return
 
+  // The W3C does not seem to have word on how postMessage should work when the
+  // origins do not match, so we do not do |canAccessWindow| check here since
+  // postMessage across origins is usefull and not harmful.
   if (guestContents.getURL().indexOf(targetOrigin) === 0 || targetOrigin === '*') {
     const sourceId = event.sender.id
     guestContents.send('ELECTRON_GUEST_WINDOW_POSTMESSAGE', sourceId, message, sourceOrigin)
@@ -191,5 +217,7 @@ ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_POSTMESSAGE', function (event,
 
 ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_WEB_CONTENTS_METHOD', function (event, guestId, method, ...args) {
   const guestContents = webContents.fromId(guestId)
-  if (guestContents != null) guestContents[method].apply(guestContents, args)
+  if (guestContents != null && canAccessWindow(event.sender, guestContents)) {
+    guestContents[method].apply(guestContents, args)
+  }
 })