Browse Source

fix: more security for internal comm channels

tophf 4 years ago
parent
commit
9c84d15d37

+ 38 - 16
src/injected/content/inject.js

@@ -5,8 +5,14 @@ import {
   INJECT_CONTENT, INJECT_MAPPING, INJECT_PAGE, browser,
 } from '../util';
 
+/* In FF, content scripts running in a same-origin frame cannot directly call parent's functions
+ * so we'll use the extension's UUID, which is unique per computer in FF, for messages
+ * like VAULT_WRITER to avoid interception by sites that can add listeners for all of our
+ * INIT_FUNC_NAME ids even though we change it now with each release. */
 const INIT_FUNC_NAME = process.env.INIT_FUNC_NAME;
-const VAULT_SEED_NAME = INIT_FUNC_NAME + process.env.VAULT_ID_NAME;
+const VM_UUID = browser.runtime.getURL('');
+const VAULT_SEED_NAME = `${VM_UUID}VS`;
+const VAULT_WRITER = `${VM_UUID}VW`;
 let contLists;
 let pgLists;
 /** @type {Object<string,VMInjectionRealm>} */
@@ -28,16 +34,25 @@ defineProperty(window, INIT_FUNC_NAME, {
   enumerable: false,
   writable: false,
 });
-window::on(INIT_FUNC_NAME, evt => {
-  if (!frameEventWnd) {
-    // setupVaultId's first event is the frame's contentWindow
-    frameEventWnd = evt::getRelatedTarget();
-  } else {
-    // setupVaultId's second event is the vaultId
-    bridge.post('WriteVault', evt::getDetail(), INJECT_PAGE, frameEventWnd);
-    frameEventWnd = null;
-  }
-});
+if (IS_FIREFOX) {
+  window::on(VAULT_WRITER, evt => {
+    evt::stopImmediatePropagation();
+    if (!frameEventWnd) {
+      // setupVaultId's first event is the frame's contentWindow
+      frameEventWnd = evt::getRelatedTarget();
+    } else {
+      // setupVaultId's second event is the vaultId
+      tellBridgeToWriteVault(evt::getDetail(), frameEventWnd);
+      frameEventWnd = null;
+    }
+  }, true);
+} else {
+  safeDefineProperty(global, VAULT_WRITER, {
+    configurable: false,
+    value: tellBridgeToWriteVault,
+  });
+}
+
 bridge.addHandlers({
   /**
    * FF bug workaround to enable processing of sourceURL in injected page scripts
@@ -81,7 +96,7 @@ export function injectPageSandbox(contentId, webId) {
   window::on(handshakeId, handshaker, { capture: true, once: true });
   inject({
     code: `(${VMInitInjection}(${IS_FIREFOX},'${handshakeId}','${vaultId}'))()`
-      + `\n//# sourceURL=${browser.runtime.getURL('sandbox/injected-web.js')}`,
+      + `\n//# sourceURL=${VM_UUID}sandbox/injected-web.js`,
   });
   // Clean up in case CSP prevented the script from running
   window::off(handshakeId, handshaker, true);
@@ -198,7 +213,7 @@ function inject(item) {
   if (IS_FIREFOX) {
     onError = e => {
       const { stack } = e.error;
-      if (!stack || `${stack}`.includes(browser.runtime.getURL(''))) {
+      if (!stack || `${stack}`.includes(VM_UUID)) {
         log('error', [item.displayName], e.error);
         e.preventDefault();
       }
@@ -264,9 +279,16 @@ function setupContentInvoker(contentId, webId) {
 }
 
 function tellParentToWriteVault(parent, vaultId) {
-  // In FF, content scripts running in a same-origin frame cannot directly call parent's functions
   // TODO: Use a single PointerEvent with `pointerType: vaultId` when strict_min_version >= 59
-  parent::fire(new MouseEventSafe(INIT_FUNC_NAME, { relatedTarget: window }));
-  parent::fire(new CustomEventSafe(INIT_FUNC_NAME, { detail: vaultId }));
+  if (IS_FIREFOX) {
+    parent::fire(new MouseEventSafe(VAULT_WRITER, { relatedTarget: window }));
+    parent::fire(new CustomEventSafe(VAULT_WRITER, { detail: vaultId }));
+  } else {
+    parent[VAULT_WRITER](vaultId, window);
+  }
   return vaultId;
 }
+
+function tellBridgeToWriteVault(vaultId, wnd) {
+  bridge.post('WriteVault', vaultId, INJECT_PAGE, wnd);
+}

+ 1 - 0
src/injected/content/safe-globals-content.js

@@ -39,6 +39,7 @@ export const { random: mathRandom } = Math;
 export const regexpTest = RegExp[PROTO].test;
 export const { toStringTag } = Symbol; // used by ProtectWebpackBootstrapPlugin
 export const { decode: tdDecode } = TextDecoderSafe[PROTO];
+export const { stopImmediatePropagation } = Event[PROTO];
 export const { get: getHref } = describeProperty(HTMLAnchorElement[PROTO], 'href');
 export const getDetail = describeProperty(CustomEventSafe[PROTO], 'detail').get;
 export const getRelatedTarget = describeProperty(MouseEventSafe[PROTO], 'relatedTarget').get;

+ 1 - 0
src/injected/safe-globals-injected.js

@@ -49,6 +49,7 @@ export const vmOwnFuncToString = () => '[Violentmonkey property]';
 /** Using __proto__ because Object.create(null) may be spoofed */
 export const createNullObj = () => ({ __proto__: null });
 
+// WARNING! `obj` must use __proto__:null
 export const ensureNestedProp = (obj, bucketId, key, defaultValue) => {
   const bucket = obj[bucketId] || (
     obj[bucketId] = createNullObj()

+ 2 - 1
src/injected/util/index.js

@@ -22,6 +22,7 @@ export const bindEvents = (srcId, destId, bridge, cloneInto) => {
    * whereas MouseEvent (and some others) can't transfer objects without stringification. */
   let incomingNodeEvent;
   window::on(srcId, e => {
+    e::stopImmediatePropagation();
     if (process.env.DEBUG) {
       console.info(`[bridge.${bridge.ids ? 'host' : 'guest.web'}] received`,
         incomingNodeEvent ? e::getRelatedTarget() : e::getDetail());
@@ -37,7 +38,7 @@ export const bindEvents = (srcId, destId, bridge, cloneInto) => {
       bridge.onHandle(incomingNodeEvent);
       incomingNodeEvent = null;
     }
-  });
+  }, true);
   bridge.post = (cmd, data, { dataKey } = bridge, node) => {
     // Constructing the event now so we don't send anything if it throws on invalid `node`
     const evtNode = node && new MouseEventSafe(destId, { __proto__: null, relatedTarget: node });

+ 1 - 1
src/injected/web/index.js

@@ -25,7 +25,7 @@ export default function initialize(
       e = e::getDetail();
       webId = e[0];
       contentId = e[1];
-    }, { once: true });
+    }, { __proto__: null, once: true, capture: true });
     window::fire(new CustomEventSafe(process.env.HANDSHAKE_ID));
   }
   bridge.dataKey = contentId;

+ 2 - 0
src/injected/web/safe-globals-web.js

@@ -63,6 +63,7 @@ export let
   parseFromString, // DOMParser
   readAsDataURL, // FileReader
   safeResponseBlob, // Response - safe = "safe global" to disambiguate the name
+  stopImmediatePropagation,
   then,
   // various getters
   getBlobType, // Blob
@@ -144,6 +145,7 @@ export const VAULT = (() => {
     parseFromString = res[i += 1] || DOMParserSafe[PROTO].parseFromString,
     readAsDataURL = res[i += 1] || FileReaderSafe[PROTO].readAsDataURL,
     safeResponseBlob = res[i += 1] || ResponseSafe[PROTO].blob,
+    stopImmediatePropagation = res[i += 1] || Event[PROTO].stopImmediatePropagation,
     then = res[i += 1] || PromiseSafe[PROTO].then,
     // various getters
     getBlobType = res[i += 1] || describeProperty(BlobSafe[PROTO], 'type').get,