fix: remove more unsafe calls

+ improve callstack in error messages
+ reuse 'prototype'

.eslintrc.js

@@ -10,12 +10,15 @@ const unsafeSharedEnvironment = [
   'src/common/object.js',
   'src/common/util.js',
 ];
+const unsafeAndSharedEnvironment = [
+  ...unsafeEnvironment,
+  ...unsafeSharedEnvironment,
+];
 const commonGlobals = getGlobals('src/common/safe-globals.js');
 const injectedGlobals = {
   ...commonGlobals,
   ...getGlobals('src/injected/safe-injected-globals.js'),
 };
-
 module.exports = {
   root: true,
   extends: [
@@ -31,14 +34,19 @@ module.exports = {
     // `browser` is a local variable since we remove the global `chrome` and `browser` in injected*
     // to prevent exposing them to userscripts with `@inject-into content`
     files: ['*'],
-    excludedFiles: [...unsafeEnvironment, ...unsafeSharedEnvironment],
+    excludedFiles: unsafeAndSharedEnvironment,
     globals: {
       browser: false,
       ...commonGlobals,
     },
+  }, {
+    files: unsafeSharedEnvironment,
+    globals: commonGlobals,
   }, {
     files: unsafeEnvironment,
     globals: injectedGlobals,
+  }, {
+    files: unsafeAndSharedEnvironment,
     rules: {
       // Whitelisting our safe globals
       'no-restricted-globals': ['error',
@@ -56,6 +64,9 @@ module.exports = {
       }, {
         selector: 'ArrayPattern',
         message: 'Array destructuring in an unsafe environment',
+      }, {
+        selector: 'CallExpression > SpreadElement',
+        message: 'Array spreading in an unsafe environment',
       }],
     },
   }, {

scripts/webpack.conf.js

@@ -14,6 +14,7 @@ const INIT_FUNC_NAME = 'VMInitInjection';
 const VM_VER = require('../package.json').version.replace(/-[^.]*/, '');
 const WEBPACK_OPTS = {
   node: {
+    global: false,
     process: false,
     setImmediate: false,
   },

src/background/sync/base.js

@@ -3,7 +3,7 @@ import {
 } from '#/common';
 import { TIMEOUT_HOUR } from '#/common/consts';
 import {
-  forEachEntry, objectSet, objectPick, objectPurify,
+  forEachEntry, objectSet, objectPick,
 } from '#/common/object';
 import {
   getEventEmitter, getOption, setOption, hookOptions,
@@ -175,6 +175,19 @@ function parseScriptData(raw) {
   return data;
 }
 
+function objectPurify(obj) {
+  // Remove keys with undefined values
+  if (Array.isArray(obj)) {
+    obj.forEach(objectPurify);
+  } else if (obj && typeof obj === 'object') {
+    obj::forEachEntry(([key, value]) => {
+      if (typeof value === 'undefined') delete obj[key];
+      else objectPurify(value);
+    });
+  }
+  return obj;
+}
+
 function serviceFactory(base) {
   const Service = function constructor() {
     this.initialize();

src/common/browser.js

@@ -5,7 +5,8 @@
 if (!global.browser?.runtime?.sendMessage) {
   // region Chrome
   const { chrome, Error, Promise, Proxy } = global;
-  const { bind } = Proxy;
+  const MESSAGE = 'message';
+  const STACK = 'stack';
   /** onXXX like onMessage */
   const isApiEvent = key => key[0] === 'o' && key[1] === 'n';
   /** API types or enums or literal constants */
@@ -47,60 +48,72 @@ if (!global.browser?.runtime?.sendMessage) {
         reject = _reject;
       });
       // Make the error messages actually useful by capturing a real stack
-      const stackInfo = new Error();
-      // Using (...results) for API callbacks that return several results (we don't use them though)
-      thisArg::func(...args, (...results) => {
-        let err = chrome.runtime.lastError;
-        let isRuntime;
-        if (err) {
-          err = err.message;
-          isRuntime = true;
-        } else if (preprocessorFunc) {
-          err = preprocessorFunc(resolve, ...results);
-        } else {
-          resolve(results[0]);
-        }
+      const stackInfo = new Error(`callstack before invoking ${func.name || 'chrome API'}:`);
+      // A single parameter `result` is fine because we don't use API that return more
+      args[args.length] = result => {
+        const runtimeErr = chrome.runtime.lastError;
+        const err = runtimeErr || (
+          preprocessorFunc
+            ? preprocessorFunc(resolve, result)
+            : resolve(result)
+        );
         // Prefer `reject` over `throw` which stops debugger in 'pause on exceptions' mode
         if (err) {
-          // Using \n\n so the calling code can strip the stack easily if needed
-          err = new Error(`${err}\n\n${stackInfo.stack}`);
-          err.isRuntime = isRuntime;
-          reject(err);
+          if (!runtimeErr) stackInfo[STACK] = `${err[1]}\n${stackInfo[STACK]}`;
+          stackInfo[MESSAGE] = runtimeErr ? err[MESSAGE] : `${err[0]}`;
+          stackInfo.isRuntime = !!runtimeErr;
+          reject(stackInfo);
         }
-      });
-      if (process.env.DEBUG) promise.catch(err => console.warn(args, err?.message || err));
+      };
+      func::apply(thisArg, args);
+      if (process.env.DEBUG) promise.catch(err => console.warn(args, err?.[MESSAGE] || err));
       return promise;
     }
   );
+  // Both result and error must be explicitly specified to avoid prototype eavesdropping
+  const wrapSuccess = result => [
+    result,
+    null,
+  ];
+  // Both result and error must be explicitly specified to avoid prototype eavesdropping
+  const wrapError = err => process.env.DEBUG && console.warn(err) || [
+    null,
+    err instanceof Error
+      ? [err[MESSAGE], err[STACK]]
+      : [err, ''],
+  ];
   const sendResponseAsync = async (result, sendResponse) => {
     try {
       result = await result;
       if (process.env.DEBUG) console.info('send', result);
-      sendResponse({ data: result });
+      sendResponse(wrapSuccess(result));
     } catch (err) {
-      if (process.env.DEBUG) console.warn(err);
-      sendResponse({ error: err instanceof Error ? err.stack : err });
+      sendResponse(wrapError(err));
     }
   };
   const onMessageListener = (listener, message, sender, sendResponse) => {
     if (process.env.DEBUG) console.info('receive', message);
-    const result = listener(message, sender);
-    if (result && isFunction(result.then)) {
-      sendResponseAsync(result, sendResponse);
-      return true;
-    }
-    if (result !== undefined) {
+    try {
+      const result = listener(message, sender);
+      if (result instanceof Promise) {
+        sendResponseAsync(result, sendResponse);
+        return true;
+      }
       // In some browsers (e.g Chrome 56, Vivaldi), the listener in
       // popup pages are not properly cleared after closed.
       // They may send `undefined` before the real response is sent.
-      sendResponse({ data: result });
+      if (result !== undefined) {
+        sendResponse(wrapSuccess(result));
+      }
+    } catch (err) {
+      sendResponse(wrapError(err));
     }
   };
   /** @returns {?} error */
   const unwrapResponse = (resolve, response) => (
     !response && 'null response'
-    || response.error
-    || resolve(response.data)
+    || response[1] // error created in wrapError
+    || resolve(response[0]) // result created in wrapSuccess
   );
   const wrapSendMessage = (runtime, sendMessage) => (
     wrapAsync(runtime, sendMessage, unwrapResponse)
@@ -131,6 +144,7 @@ if (!global.browser?.runtime?.sendMessage) {
   // endregion
 } else if (process.env.DEBUG && !global.chrome.app) {
   // region Firefox
+  /* eslint-disable no-restricted-syntax */// this is a debug-only section
   let counter = 0;
   const { runtime } = global.browser;
   const { sendMessage, onMessage } = runtime;
@@ -159,5 +173,6 @@ if (!global.browser?.runtime?.sendMessage) {
     .then(data => log('on', [data], id, true), console.warn);
     return result;
   });
+  /* eslint-enable no-restricted-syntax */
   // endregion
 }

src/common/index.js

@@ -59,14 +59,16 @@ const COMMANDS_WITH_SRC = [
   'SetPopup',
 */
 ];
+// Used in safe context
+// eslint-disable-next-line no-restricted-syntax
+const getBgPage = () => browser.extension.getBackgroundPage?.();
 
 /**
  * Sends the command+data directly so it's synchronous and faster than sendCmd thanks to deepCopy.
  * WARNING! Make sure `cmd` handler doesn't use `src` or `cmd` is listed in COMMANDS_WITH_SRC.
  */
 export function sendCmdDirectly(cmd, data, options) {
-  const bg = !COMMANDS_WITH_SRC.includes(cmd)
-    && browser.extension.getBackgroundPage?.();
+  const bg = !COMMANDS_WITH_SRC.includes(cmd) && getBgPage();
   return bg && bg !== window && bg.deepCopy
     ? bg.handleCommandMessage(bg.deepCopy({ cmd, data })).then(deepCopy)
     : sendCmd(cmd, data, options);
@@ -87,7 +89,7 @@ export function sendTabCmd(tabId, cmd, data, options) {
 export function sendMessage(payload, { retry, ignoreError } = {}) {
   if (retry) return sendMessageRetry(payload);
   let promise = browser.runtime.sendMessage(payload);
-  if (ignoreError || window === browser.extension.getBackgroundPage?.()) {
+  if (ignoreError || window === getBgPage()) {
     promise = promise.catch(noop);
   }
   return promise;

src/common/object.js

@@ -46,19 +46,6 @@ export function objectSet(obj, rawKey, val) {
   return root;
 }
 
-export function objectPurify(obj) {
-  // Remove keys with undefined values
-  if (Array.isArray(obj)) {
-    obj.forEach(objectPurify);
-  } else if (obj && typeof obj === 'object') {
-    obj::forEachEntry(([key, value]) => {
-      if (typeof value === 'undefined') delete obj[key];
-      else objectPurify(value);
-    });
-  }
-  return obj;
-}
-
 /**
  * @param {{}} obj
  * @param {string[]} keys
@@ -101,6 +88,8 @@ export function forEachValue(func) {
 export function deepCopy(src) {
   return src && (
     Array.isArray(src) && src.map(deepCopy)
+    // Used in safe context
+    // eslint-disable-next-line no-restricted-syntax
     || typeof src === 'object' && src::mapEntry(([, val]) => deepCopy(val))
   ) || src;
 }

src/common/safe-globals.js

@@ -1,6 +1,6 @@
 /* eslint-disable no-unused-vars */
 
-const globalThis = (function _() { return this || {}; }());
+const global = (function _() { return this || {}; }());
 // Not exporting the built-in globals because this also runs in node
 const {
   Array, Boolean, Object, Promise, Uint8Array,
@@ -9,6 +9,7 @@ const {
      https://html.spec.whatwg.org/multipage/window-object.html#dom-document-dev */
   document,
   window,
-} = globalThis;
+} = global;
 export const { hasOwnProperty } = {};
-export const safeCall = hasOwnProperty.call.bind(hasOwnProperty.call);
+export const { apply, bind, call } = hasOwnProperty;
+export const safeCall = call.bind(call);

src/common/util.js

@@ -21,6 +21,8 @@ export function toString(param) {
 export function memoize(func, resolver = toString) {
   const cacheMap = {};
   function memoized(...args) {
+    // Used in safe context
+    // eslint-disable-next-line no-restricted-syntax
     const key = resolver(...args);
     let cache = cacheMap[key];
     if (!cache) {
@@ -140,7 +142,10 @@ const DIGITS_RE = /^\d+$/; // using regexp to avoid +'1e2' being parsed as 100
 
 /** @return -1 | 0 | 1 */
 export function compareVersion(ver1, ver2) {
+  // Used in safe context
+  // eslint-disable-next-line no-restricted-syntax
   const [, main1 = ver1 || '', pre1] = VERSION_RE.exec(ver1);
+  // eslint-disable-next-line no-restricted-syntax
   const [, main2 = ver2 || '', pre2] = VERSION_RE.exec(ver2);
   const delta = compareVersionChunk(main1, main2)
     || !pre1 - !pre2 // 1.2.3-pre-release is less than 1.2.3
@@ -253,9 +258,9 @@ const FORCED_ACCEPT = {
   'greasyfork.org': 'application/javascript, text/plain, text/css',
 };
 /** @typedef {{
-  url: string
-  status: number
-  headers: Headers
+  url: string,
+  status: number,
+  headers: Headers,
   data: string|ArrayBuffer|Blob|Object
 }} VMRequestResponse */
 /**

src/injected/content/clipboard.js

@@ -4,9 +4,9 @@ import bridge from './bridge';
 
 // old Firefox defines it on a different prototype so we'll just grab it from document directly
 const { execCommand } = document;
-const { setData } = DataTransfer.prototype;
-const { get: getClipboardData } = describeProperty(ClipboardEvent.prototype, 'clipboardData');
-const { preventDefault, stopImmediatePropagation } = Event.prototype;
+const { setData } = DataTransfer[Prototype];
+const { get: getClipboardData } = describeProperty(ClipboardEvent[Prototype], 'clipboardData');
+const { preventDefault, stopImmediatePropagation } = Event[Prototype];
 
 let clipboardData;
 

src/injected/content/index.js

@@ -21,10 +21,8 @@ let pendingSetPopup;
 const { split } = '';
 
 (async () => {
-  const eventIds = [
-    bridge.contentId = getUniqId(),
-    bridge.webId = getUniqId(),
-  ];
+  const contentId = getUniqId();
+  const webId = getUniqId();
   // injecting right now before site scripts can mangle globals or intercept our contentId
   // except for XML documents as their appearance breaks, but first we're sending
   // a request for the data because injectPageSandbox takes ~5ms
@@ -34,20 +32,20 @@ const { split } = '';
     IS_FIREFOX && global.location.href,
     { retry: true });
   const isXml = document instanceof XMLDocument;
-  if (!isXml) injectPageSandbox();
+  if (!isXml) injectPageSandbox(contentId, webId);
   // detecting if browser.contentScripts is usable, it was added in FF59 as well as composedPath
-  const data = IS_FIREFOX && Event.prototype.composedPath
+  const data = IS_FIREFOX && Event[Prototype].composedPath
     ? await getDataFF(dataPromise)
     : await dataPromise;
   // 1) bridge.post may be overridden in injectScripts
   // 2) cloneInto is provided by Firefox in content scripts to expose data to the page
-  bridge.post = bindEvents(...eventIds, bridge.onHandle, global.cloneInto);
+  bridge.post = bindEvents(contentId, webId, bridge.onHandle, global.cloneInto);
   bridge.ids = data.ids;
   bridge.isFirefox = data.info.isFirefox;
   bridge.injectInto = data.injectInto;
-  if (data.scripts) injectScripts(data, isXml);
-  if (data.expose) bridge.post('Expose');
   isPopupShown = data.isPopupShown;
+  if (data.expose) bridge.post('Expose');
+  if (data.scripts) await injectScripts(contentId, webId, data, isXml);
   sendSetPopup();
 })().catch(IS_FIREFOX && console.error); // Firefox can't show exceptions in content scripts
 

src/injected/content/inject.js

@@ -65,19 +65,21 @@ export function appendToRoot(node) {
   return root && root::appendChild(node);
 }
 
-export function injectPageSandbox() {
+export function injectPageSandbox(contentId, webId) {
   inject({
-    code: `(${VMInitInjection}())('${bridge.webId}','${bridge.contentId}')\n//# sourceURL=${
+    code: `(${VMInitInjection}())('${webId}','${contentId}')\n//# sourceURL=${
       browser.runtime.getURL('sandbox/injected-web.js')
     }`,
   });
 }
 
 /**
+ * @param {string} contentId
+ * @param {string} webId
  * @param {VMGetInjectedData} data
  * @param {boolean} isXml
  */
-export async function injectScripts(data, isXml) {
+export async function injectScripts(contentId, webId, data, isXml) {
   const { hasMore, info } = data;
   pageInjectable = isXml ? false : null;
   realms = {
@@ -117,20 +119,20 @@ export async function injectScripts(data, isXml) {
     pageInjectable: pageInjectable ?? (hasMore && checkInjectable()),
   });
   // saving while safe
-  const getReadyState = hasMore && describeProperty(Document.prototype, 'readyState').get;
+  const getReadyState = hasMore && describeProperty(Document[Prototype], 'readyState').get;
   const hasInvoker = realms[INJECT_CONTENT].is;
   if (hasInvoker) {
-    setupContentInvoker();
+    setupContentInvoker(contentId, webId);
   }
   // Using a callback to avoid a microtask tick when the root element exists or appears.
-  onElement('*', async () => {
+  await onElement('*', async () => {
     injectAll('start');
     const onBody = (pgLists.body[0] || contLists.body[0])
       && onElement('body', injectAll, 'body');
     // document-end, -idle
     if (hasMore) {
       data = await moreData;
-      if (data) await injectDelayedScripts(data, getReadyState, hasInvoker);
+      if (data) await injectDelayedScripts(!hasInvoker && contentId, webId, data, getReadyState);
     }
     if (onBody) {
       await onBody;
@@ -141,7 +143,7 @@ export async function injectScripts(data, isXml) {
   });
 }
 
-async function injectDelayedScripts({ cache, scripts }, getReadyState, hasInvoker) {
+async function injectDelayedScripts(contentId, webId, { cache, scripts }, getReadyState) {
   realms::forEachKey(r => {
     realms[r].info.cache = cache;
   });
@@ -163,8 +165,8 @@ async function injectDelayedScripts({ cache, scripts }, getReadyState, hasInvoke
       window::addEventListener('DOMContentLoaded', resolve, { once: true });
     });
   }
-  if (needsInvoker && !hasInvoker) {
-    setupContentInvoker();
+  if (needsInvoker && contentId) {
+    setupContentInvoker(contentId, webId);
   }
   injectAll('end');
   injectAll('idle');
@@ -234,19 +236,19 @@ async function injectList(runAt) {
 /**
  * @param {string} tag
  * @param {function} cb - callback runs immediately, unlike a chained then()
- * @param {?} [args]
+ * @param {?} [arg]
  * @returns {Promise<void>}
  */
-function onElement(tag, cb, ...args) {
+function onElement(tag, cb, arg) {
   return new Promise(resolve => {
     if (elemByTag(tag)) {
-      cb(...args);
+      cb(arg);
       resolve();
     } else {
       const observer = new MutationObserver(() => {
         if (elemByTag(tag)) {
           observer.disconnect();
-          cb(...args);
+          cb(arg);
           resolve();
         }
       });
@@ -256,8 +258,8 @@ function onElement(tag, cb, ...args) {
   });
 }
 
-function setupContentInvoker() {
-  const invokeContent = VMInitInjection()(bridge.webId, bridge.contentId, bridge.onHandle);
+function setupContentInvoker(contentId, webId) {
+  const invokeContent = VMInitInjection()(webId, contentId, bridge.onHandle);
   const postViaBridge = bridge.post;
   bridge.post = (cmd, params, realm) => (
     (realm === INJECT_CONTENT ? invokeContent : postViaBridge)(cmd, params)

src/injected/content/requests.js

@@ -2,7 +2,7 @@ import { sendCmd } from '#/common';
 import bridge from './bridge';
 
 const { fetch } = global;
-const { arrayBuffer: getArrayBuffer, blob: getBlob } = Response.prototype;
+const { arrayBuffer: getArrayBuffer, blob: getBlob } = Response[Prototype];
 
 const requests = createNullObj();
 

src/injected/index.js

@@ -13,7 +13,7 @@ if (!global.chrome.app
       fetch,
       history,
       document: { referrer },
-      Response: { prototype: { text: getText } },
+      Response: { [Prototype]: { text: getText } },
       location: { href: url },
     } = global;
     const fetchOpts = { mode: 'same-origin' };

src/injected/safe-injected-globals.js

@@ -1,12 +1,14 @@
 /* eslint-disable no-unused-vars */
 
 // Not exporting the built-in globals because this also runs in node
-const { CustomEvent, Error, dispatchEvent } = globalThis;
+// Not using destructuring to allow removal of unused variables during minification
+const { CustomEvent, Error, dispatchEvent } = global;
+export const Prototype = 'prototype';
 export const { createElementNS, getElementsByTagName } = document;
-export const { then } = Promise.prototype;
-export const { filter, forEach, includes, join, map, push } = [];
+export const { then } = Promise[Prototype];
+export const { filter, forEach, includes, indexOf, join, map, push } = [];
 export const { charCodeAt, slice, replace } = '';
-export const { append, appendChild, remove, setAttribute } = Element.prototype;
+export const { append, appendChild, remove, setAttribute } = Element[Prototype];
 export const { toString: objectToString } = {};
 export const {
   assign,

src/injected/utils/helpers.js

@@ -51,9 +51,8 @@ export function jsonDump(value) {
   return `"${value::replace(escRE, escFunc)}"`;
 }
 
-export function log(level, tags, ...args) {
-  const tagList = ['Violentmonkey'];
-  if (tags) tagList::push(...tags);
-  const prefix = tagList::map(tag => `[${tag}]`)::join('');
-  logging[level](prefix, ...args);
+/** args is [tags?, ...rest] */
+export function log(level, ...args) {
+  args[0] = `[Violentmonkey]${args[0] ? `[${args[0]::join('][')}]` : ''}`;
+  logging[level]::apply(logging, args);
 }

src/injected/utils/index.js

@@ -1,5 +1,5 @@
 export function bindEvents(srcId, destId, handle, cloneInto) {
-  const getDetail = describeProperty(CustomEvent.prototype, 'detail').get;
+  const getDetail = describeProperty(CustomEvent[Prototype], 'detail').get;
   const pageContext = cloneInto && document.defaultView;
   document::addEventListener(srcId, e => handle(e::getDetail()));
   return (cmd, params) => {

src/injected/web/gm-api.js

@@ -10,14 +10,14 @@ import { jsonDump, log, logging, NS_HTML, elemByTag } from '../utils/helpers';
 
 const {
   Blob, MouseEvent, TextDecoder,
-  RegExp: { prototype: { test } },
-  TextDecoder: { prototype: { decode: tdDecode } },
+  RegExp: { [Prototype]: { test } },
+  TextDecoder: { [Prototype]: { decode: tdDecode } },
   URL: { createObjectURL, revokeObjectURL },
 } = global;
-const { findIndex, indexOf } = [];
+const { findIndex } = [];
 const { lastIndexOf } = '';
 const { dispatchEvent, getElementById } = document;
-const { removeAttribute } = Element.prototype;
+const { removeAttribute } = Element[Prototype];
 
 const vmOwnFuncToString = () => '[Violentmonkey property]';
 export const vmOwnFunc = (func, toString) => {

src/injected/web/gm-wrapper.js

@@ -6,13 +6,10 @@ const {
   Proxy,
   Set, // 2x-3x faster lookup than object::has
   Symbol: { toStringTag, iterator: iterSym },
-  Map: { prototype: { get: mapGet, has: mapHas, [iterSym]: mapIter } },
-  Set: { prototype: { delete: setDelete, has: setHas, [iterSym]: setIter } },
+  Map: { [Prototype]: { get: mapGet, has: mapHas, [iterSym]: mapIter } },
+  Set: { [Prototype]: { delete: setDelete, has: setHas, [iterSym]: setIter } },
   Object: { getOwnPropertyNames, getOwnPropertySymbols },
 } = global;
-/** A bound function won't be stepped-into when debugging.
- * Destructuring a random function reference instead of the long `Function.prototype` */
-const { apply, bind } = Proxy;
 const { concat, slice: arraySlice } = [];
 const { startsWith } = '';
 /** Name in Greasemonkey4 -> name in GM */
@@ -154,8 +151,8 @@ if (global.wrappedJSObject) {
   globalKeys.push('wrappedJSObject');
 }
 const inheritedKeys = new Set([
-  ...getOwnPropertyNames(EventTarget.prototype),
-  ...getOwnPropertyNames(Object.prototype),
+  ...getOwnPropertyNames(EventTarget[Prototype]),
+  ...getOwnPropertyNames(Object[Prototype]),
 ]);
 inheritedKeys.has = setHas;
 

src/injected/web/requests.js

@@ -7,10 +7,10 @@ const idMap = {};
 
 export const { atob } = global;
 const { Blob, DOMParser, FileReader, Response } = global;
-const { parseFromString } = DOMParser.prototype;
-const { blob: resBlob } = Response.prototype;
-const { get: getHref } = describeProperty(HTMLAnchorElement.prototype, 'href');
-const { readAsDataURL } = FileReader.prototype;
+const { parseFromString } = DOMParser[Prototype];
+const { blob: resBlob } = Response[Prototype];
+const { get: getHref } = describeProperty(HTMLAnchorElement[Prototype], 'href');
+const { readAsDataURL } = FileReader[Prototype];
 
 bridge.addHandlers({
   __proto__: null, // Object.create(null) may be spoofed