fix: guard against hooking toString, toStringTag

* use safe toString for Number, URL, Location in string templates with sensitive data
* don't use Symbol.toStringTag and check FormData directly
* refactor: combine createNullObj and pickIntoThis

src/injected/content/gm-api-content.js

@@ -66,7 +66,7 @@ export async function sendSetPopup(isDelayed) {
       await setPopupThrottle;
       setPopupThrottle = null;
     }
-    sendCmd('SetPopup', { menus, __proto__: null }::pickIntoThis(bridge, [
+    sendCmd('SetPopup', createNullObj({ menus }, bridge, [
       'ids',
       'injectInto',
       'runningIds',

src/injected/content/index.js

@@ -33,7 +33,7 @@ async function init() {
       : await dataPromise
   );
   const { allowCmd } = bridge;
-  bridge::pickIntoThis(data, [
+  createNullObj(bridge, data, [
     'ids',
     'injectInto',
   ]);

src/injected/content/requests.js

@@ -19,11 +19,10 @@ let downloadChain = promiseResolve();
 // TODO: extract all prop names used across files into consts.js to ensure sameness
 bridge.addHandlers({
   async HttpRequest(msg, realm) {
-    requests[msg.id] = {
-      __proto__: null,
+    requests[msg.id] = createNullObj({
       realm,
       wantsBlob: msg.xhrType === 'blob',
-    }::pickIntoThis(msg, [
+    }, msg, [
       'eventsToNotify',
       'fileName',
     ]);
@@ -113,7 +112,7 @@ async function revokeBlobAfterTimeout(url) {
 
 /** ArrayBuffer/Blob in Chrome incognito is transferred in string chunks */
 function receiveAllChunks(req, msg) {
-  req::pickIntoThis(msg, ['dataSize', 'contentType']);
+  createNullObj(req, msg, ['dataSize', 'contentType']);
   req.arr = new SafeUint8Array(req.dataSize);
   processChunk(req, msg.data.response, 0);
   return !req.gotChunks

src/injected/content/safe-globals-content.js

@@ -46,7 +46,7 @@ export const getDetail = describeProperty(SafeCustomEvent[PROTO], 'detail').get;
 export const getRelatedTarget = describeProperty(SafeMouseEvent[PROTO], 'relatedTarget').get;
 export const getReadyState = describeProperty(Document[PROTO], 'readyState').get;
 export const isDocumentLoading = () => !/^(inter|compl)/::regexpTest(document::getReadyState());
-export const logging = assign(createNullObj(), console);
+export const logging = createNullObj(console);
 export const { chrome } = global;
 export const IS_FIREFOX = !chrome.app;
 export const VM_UUID = chrome.runtime.getURL('');

src/injected/safe-globals-injected.js

@@ -19,13 +19,9 @@ export const WINDOW_CLOSE = 'window.close';
 export const WINDOW_FOCUS = 'window.focus';
 export const NS_HTML = 'http://www.w3.org/1999/xhtml';
 export const CALLBACK_ID = '__CBID';
-
-export const getObjectTypeTag = val => {
-  // objectToString may call @@toStringTag getter which may throw
-  try {
-    return val && val::objectToString()::slice(8, -1);
-  } catch (e) { /* NOP */ }
-};
+/** These toString are used to avoid leaking data when converting into a string */
+const { toString: numberToString } = 0;
+const { toString: URLToString } = URL[PROTO];
 
 export const isFunction = val => typeof val === 'function';
 export const isObject = val => val !== null && typeof val === 'object';
@@ -59,8 +55,33 @@ export const setOwnProp = (obj, key, value, mutable = true) => (
 
 export const vmOwnFuncToString = () => '[Violentmonkey property]';
 
-/** Using __proto__ because Object.create(null) may be spoofed */
-export const createNullObj = () => ({ __proto__: null });
+/**
+ * Helps avoid interception via `Object.prototype`.
+ * @param {Object} [dst] - target object to clear the prototype or to pick into
+ * @param {Object} [src] - source object to pick from
+ * @param {string[]} [keys] - all keys will be picked otherwise
+ * @returns {Object} `dst` if it's already without prototype, a new object otherwise
+ */
+export const createNullObj = (dst, src, keys) => {
+  const empty = (!dst || dst.__proto__) && { __proto__: null }; // eslint-disable-line no-proto
+  if (!dst) {
+    dst = empty;
+  } else if (empty) {
+    dst = assign(empty, dst);
+  }
+  if (src) {
+    if (keys) {
+      keys::forEach(key => {
+        if (src::hasOwnProperty(key)) {
+          dst[key] = src[key];
+        }
+      });
+    } else {
+      assign(dst, src);
+    }
+  }
+  return dst;
+};
 
 // WARNING! `obj` must use __proto__:null
 export const ensureNestedProp = (obj, bucketId, key, defaultValue) => {
@@ -79,8 +100,8 @@ export const vmOwnFunc = (func, toString) => (
   setOwnProp(func, 'toString', toString || vmOwnFuncToString, false)
 );
 
-// Avoiding the need to safe-guard a bunch of methods so we use just one
-export const safeGetUniqId = (prefix = 'VM') => `${prefix}${mathRandom()}`;
+// Using just one random() to avoid many methods in vault just for this
+export const safeGetUniqId = (prefix = 'VM') => prefix + mathRandom()::numberToString(36);
 
 /** args is [tags?, ...rest] */
 export const log = (level, ...args) => {
@@ -90,31 +111,13 @@ export const log = (level, ...args) => {
   logging[level]::apply(logging, args);
 };
 
-/**
- * Picks into `this`
- * WARNING! `this` must use __proto__:null or already have own properties on the picked keys.
- * @param {Object} obj
- * @param {string[]} keys
- * @returns {Object} same object as `this`
- */
-export function pickIntoThis(obj, keys) {
-  if (obj) {
-    keys::forEach(key => {
-      if (obj::hasOwnProperty(key)) {
-        this[key] = obj[key];
-      }
-    });
-  }
-  return this;
-}
-
 /**
  * Object.defineProperty seems to be inherently broken: it reads inherited props from desc
  * (even though the purpose of this API is to define own props) and then complains when it finds
  * invalid props like an inherited setter when you only provide `{value}`.
  */
 export const safeDefineProperty = (obj, key, desc) => (
-  defineProperty(obj, key, assign(createNullObj(), desc))
+  defineProperty(obj, key, createNullObj(desc))
 );
 
 export const safePush = (arr, val) => (

src/injected/web/gm-api-wrapper.js

@@ -36,7 +36,7 @@ export function makeGmApiWrapper(script) {
     grant.length = 0;
   }
   const { id } = script.props;
-  const resources = assign(createNullObj(), meta.resources);
+  const resources = createNullObj(meta.resources);
   /** @namespace VMInjectedScript.Context */
   const context = {
     id,

src/injected/web/gm-api.js

@@ -102,7 +102,7 @@ export function makeGmApi() {
       } else if (arg1) {
         name = arg1.name;
         onload = arg1.onload;
-        opts::pickIntoThis(arg1, [
+        createNullObj(opts, arg1, [
           'url',
           'headers',
           'timeout',
@@ -147,12 +147,9 @@ export function makeGmApi() {
       return webAddElement(null, 'style', { textContent: css, id: safeGetUniqId('VMst') }, this);
     },
     GM_openInTab(url, options) {
-      return onTabCreate(
-        isObject(options)
-          ? assign(createNullObj(), options, { url })
-          : { active: !options, url },
-        this,
-      );
+      options = createNullObj(isObject(options) ? options : { active: !options });
+      options.url = url;
+      return onTabCreate(options, this);
     },
     GM_notification(text, title, image, onclick) {
       const options = isObject(text) ? text : {

src/injected/web/gm-global-wrapper.js

@@ -4,7 +4,7 @@ import { FastLookup, safeConcat } from './util-web';
 
 /** The index strings that look exactly like integers can't be forged
  * but for example '011' doesn't look like 11 so it's allowed */
-const isFrameIndex = key => key >= 0 && key <= 0xFFFF_FFFE && key === `${+key}`;
+const isFrameIndex = key => key >= 0 && key <= 0xFFFF_FFFE && key === (+key)::numberToString();
 const scopeSym = SafeSymbol.unscopables;
 const globalKeysSet = FastLookup();
 const globalKeys = (function makeGlobalKeys() {
@@ -143,7 +143,7 @@ for (const name in unforgeables) { /* proto is null */// eslint-disable-line gua
   );
   let fn;
   if (info) {
-    info = assign(createNullObj(), info);
+    info = createNullObj(info);
     // currently only `document` and `window`
     if ((fn = info.get)) info.get = fn::bind(thisObj);
     // currently only `location`
@@ -165,7 +165,7 @@ builtinGlobals = null; // eslint-disable-line no-global-assign
  */
 export function makeGlobalWrapper(local) {
   const events = createNullObj();
-  const readonlys = assign(createNullObj(), readonlyKeys);
+  const readonlys = createNullObj(readonlyKeys);
   let globals = globalKeysSet; // will be copied only if modified
   /* Browsers may return [object Object] for Object.prototype.toString(window)
      on our `window` proxy so jQuery libs see it as a plain object and throw
@@ -254,7 +254,11 @@ function makeOwnKeys(local, globals) {
   const names = getOwnPropertyNames(local)::filter(notIncludedIn, globals);
   const symbols = getOwnPropertySymbols(local)::filter(notIncludedIn, globals);
   const frameIndexes = [];
-  for (let i = 0, s; getObjectTypeTag(global[s = `${i}`]) === 'Window'; i += 1) {
+  // No way to verify a cross-origin window is a `Window`, so let's just trust the numbers
+  for (let i = 0, s, w, len = getOwnProp(window, 'length');
+    i < len && isObject(w = window[s = i::numberToString()]) && getOwnProp(w, 'window') === w;
+    i += 1
+  ) {
     if (!(s in local)) {
       setOwnProp(frameIndexes, s, s);
     }

src/injected/web/requests.js

@@ -12,7 +12,7 @@ bridge.addHandlers({
 export function onRequestCreate(opts, context, fileName) {
   if (!opts.url) throw new SafeError('Required parameter "url" is missing.');
   const scriptId = context.id;
-  const id = safeGetUniqId(`VMxhr${scriptId}`);
+  const id = safeGetUniqId('VMxhr');
   const req = {
     __proto__: null,
     id,
@@ -95,13 +95,12 @@ function callback(req, msg) {
 
 function start(req, context, fileName) {
   const { id, scriptId } = req;
-  const opts = assign(createNullObj(), req.opts);
+  const opts = createNullObj(req.opts);
   // withCredentials is for GM4 compatibility and used only if `anonymous` is not set,
   // it's true by default per the standard/historical behavior of gmxhr
   const { data, withCredentials = true, anonymous = !withCredentials } = opts;
   idMap[id] = req;
-  bridge.post('HttpRequest', {
-    __proto__: null,
+  bridge.post('HttpRequest', createNullObj({
     id,
     scriptId,
     anonymous,
@@ -111,11 +110,8 @@ function start(req, context, fileName) {
       || (opts.binary || !isObject(data)) && [`${data}`]
       // FF56+ can send any cloneable data directly, FF52-55 can't due to https://bugzil.la/1371246
       || IS_FIREFOX && bridge.ua.browserVersion >= 56 && [data]
-      /* Chrome can't directly transfer FormData to isolated world so we explode it,
-       * trusting its iterator is usable because the only reason for a site to break it
-       * is to fight a userscript, which it can do by breaking FormData constructor anyway */
-      // eslint-disable-next-line no-restricted-syntax
-      || (getObjectTypeTag(data) === 'FormData' ? [[...data], 'fd'] : [data, 'bin']),
+      || getFormData(data)
+      || [data, 'bin'],
     eventsToNotify: [
       'abort',
       'error',
@@ -127,7 +123,7 @@ function start(req, context, fileName) {
       'timeout',
     ]::filter(key => isFunction(getOwnProp(opts, `on${key}`))),
     xhrType: getResponseType(opts.responseType),
-  }::pickIntoThis(opts, [
+  }, opts, [
     'headers',
     'method',
     'overrideMimeType',
@@ -138,6 +134,17 @@ function start(req, context, fileName) {
   ]), context);
 }
 
+/** Chrome can't directly transfer FormData to isolated world so we explode it,
+ * trusting its iterator is usable because the only reason for a site to break it
+ * is to fight a userscript, which it can do by breaking FormData constructor anyway */
+function getFormData(data) {
+  try {
+    return [[...data::formDataEntries()], 'fd']; // eslint-disable-line no-restricted-syntax
+  } catch (e) {
+    /**/
+  }
+}
+
 function getResponseType(responseType = '') {
   switch (responseType) {
   case 'arraybuffer':

src/injected/web/safe-globals-web.js

@@ -61,6 +61,7 @@ export let
   // various methods
   arrayIsArray,
   createObjectURL,
+  formDataEntries,
   funcToString,
   jsonParse,
   jsonStringify,
@@ -143,13 +144,14 @@ export const VAULT = (() => {
     safeCall = res[i += 1] || (call = SafeObject.call).bind(call),
     // various methods
     createObjectURL = res[i += 1] || src.URL.createObjectURL,
+    formDataEntries = res[i += 1] || src.FormData[PROTO].entries,
     funcToString = res[i += 1] || safeCall.toString,
     arrayIsArray = res[i += 1] || src.Array.isArray,
     /* Exporting JSON methods separately instead of exporting SafeJSON as its props may be broken
      * by the page if it gains access to any Object from the vault e.g. a thrown SafeError. */
     jsonParse = res[i += 1] || src.JSON.parse,
     jsonStringify = res[i += 1] || src.JSON.stringify,
-    logging = res[i += 1] || assign(createNullObj(), src.console),
+    logging = res[i += 1] || createNullObj(src.console),
     mathRandom = res[i += 1] || src.Math.random,
     parseFromString = res[i += 1] || SafeDOMParser[PROTO].parseFromString,
     stopImmediatePropagation = res[i += 1] || src.Event[PROTO].stopImmediatePropagation,

src/injected/web/util-web.js

@@ -67,7 +67,7 @@ export const FastLookup = (hubs = createNullObj()) => {
     clone() {
       const clone = createNullObj();
       for (const group in hubs) { /* proto is null */// eslint-disable-line guard-for-in
-        clone[group] = assign(createNullObj(), hubs[group]);
+        clone[group] = createNullObj(hubs[group]);
       }
       return FastLookup(clone);
     },