Repository: violentmonkey/violentmonkey
Pull request #1604 (2022-10-09 15:31:14)

fix: more reliable commandsForSelf check
Instead of manually maintaining the list in `commandsForSelf`, which is error-prone and easy to forget, introducing two methods  addOwnCommands and addPublicCommands, which explicitly indicate the security scope.


Patch:
diff --git a/src/background/index.js b/src/background/index.js
index 665702a6..ecf79d0d 100644
--- a/src/background/index.js
+++ b/src/background/index.js
@@ -1,10 +1,10 @@
 import '@/common/browser';
 import { getActiveTab, makePause, sendCmd } from '@/common';
-import { TIMEOUT_24HOURS, TIMEOUT_MAX, extensionOrigin } from '@/common/consts';
+import { TIMEOUT_24HOURS, TIMEOUT_MAX, extensionRoot, extensionOrigin } from '@/common/consts';
 import { deepCopy } from '@/common/object';
 import * as tld from '@/common/tld';
 import * as sync from './sync';
-import { commands } from './utils';
+import { addOwnCommands, addPublicCommands, commands } from './utils';
 import { getData, getSizes, checkRemove } from './utils/db';
 import { initialize } from './utils/init';
 import { getOption, hookOptions } from './utils/options';
@@ -26,7 +26,7 @@ hookOptions((changes) => {
   sendCmd('UpdateOptions', changes);
 });
 
-Object.assign(commands, {
+addOwnCommands({
   async GetData(opts) {
     const data = await getData(opts);
     data.sync = sync.getStates();
@@ -43,6 +43,9 @@ Object.assign(commands, {
       domain: host && tld.getDomain(host) || host,
     };
   },
+});
+
+addPublicCommands({
   /**
    * Timers in content scripts are shared with the web page so it can clear them.
    * await sendCmd('SetTimeout', 100) in injected/content
@@ -66,31 +69,30 @@ const commandsToSyncIfTruthy = [
   'CheckRemove',
   'CheckUpdate',
 ];
-const commandsForSelf = [
-  // TODO: maybe just add a prefix for all content-exposed commands?
-  ...commandsToSync,
-  ...commandsToSyncIfTruthy,
-  'ExportZip',
-  'GetAllOptions',
-  'GetData',
-  'GetSizes',
-  'GetOptions',
-  'SetOptions',
-  'SetValueStores',
-  'Storage',
-];
 
 async function handleCommandMessage({ cmd, data } = {}, src) {
-  if (src && !`${src.url}`.startsWith(extensionOrigin) && commandsForSelf.includes(cmd)) {
-    throw `Command is only allowed in extension context: ${cmd}`;
+  const func = commands::hasOwnProperty(cmd) && commands[cmd];
+  if (!func) {
+    throw new SafeError(`Unknown command: ${cmd}`);
+  }
+  // The `src` is omitted when invoked via sendCmdDirectly unless fakeSrc is set.
+  // The `origin` is Chrome-only, it can't be spoofed by a compromised tab unlike `url`.
+  if (func.isOwn && src && !src.fake
+  && (src.origin ? src.origin !== extensionOrigin : !`${src.url}`.startsWith(extensionRoot))) {
+    throw new SafeError(`Command is only allowed in extension context: ${cmd}`);
   }
-  const res = await commands[cmd]?.(data, src);
-  if (commandsToSync.includes(cmd)
-  || res && commandsToSyncIfTruthy.includes(cmd)) {
-    sync.sync();
+  try {
+    const res = await func(data, src);
+    if (commandsToSync.includes(cmd)
+    || res && commandsToSyncIfTruthy.includes(cmd)) {
+      sync.sync();
+    }
+    // `undefined` is not transferable, but `null` is
+    return res ?? null;
+  } catch (err) {
+    // Adding `stack` info + in FF a rejected Promise value is transferred only for an Error object
+    throw err instanceof SafeError ? err : new SafeError(err);
   }
-  // `undefined` is not transferable, but `null` is
-  return res ?? null;
 }
 
 function autoUpdate() {
@@ -108,13 +110,7 @@ function autoUpdate() {
 initialize(() => {
   global.handleCommandMessage = handleCommandMessage;
   global.deepCopy = deepCopy;
-  browser.runtime.onMessage.addListener(
-    IS_FIREFOX // in FF a rejected Promise value is transferred only if it's an Error object
-      ? (...args) => handleCommandMessage(...args).catch(e => (
-        Promise.reject(e instanceof Error ? e : new Error(e))
-      )) // Didn't use `throw` to avoid interruption in devtools with pause-on-exception enabled.
-      : handleCommandMessage,
-  );
+  browser.runtime.onMessage.addListener(handleCommandMessage);
   setTimeout(autoUpdate, 2e4);
   sync.initialize();
   checkRemove();
diff --git a/src/background/sync/index.js b/src/background/sync/index.js
index 395f5d40..3509aa83 100644
--- a/src/background/sync/index.js
+++ b/src/background/sync/index.js
@@ -10,9 +10,9 @@ import './dropbox';
 import './onedrive';
 import './googledrive';
 import './webdav';
-import { commands } from '../utils/message';
+import { addOwnCommands } from '../utils/message';
 
-Object.assign(commands, {
+addOwnCommands({
   SyncAuthorize: authorize,
   SyncRevoke: revoke,
   SyncStart: sync,
diff --git a/src/background/utils/cache.js b/src/background/utils/cache.js
index 6e462dd6..77a19840 100644
--- a/src/background/utils/cache.js
+++ b/src/background/utils/cache.js
@@ -1,11 +1,11 @@
 import initCache from '@/common/cache';
-import { commands } from './message';
+import { addOwnCommands } from './message';
 
 const cache = initCache({
   lifetime: 5 * 60 * 1000,
 });
 
-Object.assign(commands, {
+addOwnCommands({
   CacheLoad(data) {
     return cache.get(data) || null;
   },
diff --git a/src/background/utils/clipboard.js b/src/background/utils/clipboard.js
index e36eb5b0..d0ec80d1 100644
--- a/src/background/utils/clipboard.js
+++ b/src/background/utils/clipboard.js
@@ -1,9 +1,9 @@
-import { commands } from './message';
+import { addPublicCommands } from './message';
 
 const textarea = document.createElement('textarea');
 let clipboardData;
 
-Object.assign(commands, {
+addPublicCommands({
   SetClipboard(data) {
     clipboardData = data;
     textarea.focus();
diff --git a/src/background/utils/db.js b/src/background/utils/db.js
index f215118d..81dfe462 100644
--- a/src/background/utils/db.js
+++ b/src/background/utils/db.js
@@ -9,7 +9,7 @@ import pluginEvents from '../plugin/events';
 import { getNameURI, parseMeta, newScript, getDefaultCustom } from './script';
 import { testScript, testBlacklist, testerBatch } from './tester';
 import { preInitialize } from './init';
-import { commands } from './message';
+import { addOwnCommands, commands } from './message';
 import patchDB from './patch-db';
 import { setOption } from './options';
 import storage, {
@@ -32,7 +32,7 @@ export const store = {
   },
 };
 
-Object.assign(commands, {
+addOwnCommands({
   CheckPosition: sortScripts,
   CheckRemove: checkRemove,
   /** @return {VMScript} */
diff --git a/src/background/utils/icon.js b/src/background/utils/icon.js
index 268f96d1..5b1cdef5 100644
--- a/src/background/utils/icon.js
+++ b/src/background/utils/icon.js
@@ -2,17 +2,20 @@ import { i18n, makeDataUri, noop } from '@/common';
 import { BLACKLIST, ICON_PREFIX, INJECTABLE_TAB_URL_RE } from '@/common/consts';
 import { objectPick } from '@/common/object';
 import { postInitialize } from './init';
-import { commands, forEachTab } from './message';
+import { addOwnCommands, addPublicCommands, forEachTab } from './message';
 import { getOption, hookOptions } from './options';
 import { testBlacklist } from './tester';
 import storage from './storage';
 
-Object.assign(commands, {
+addOwnCommands({
   GetImageData: async url => (
     url.startsWith(ICON_PREFIX)
       ? (await getOwnIcon(url)).uri
       : (await storage.cache.fetch(url), makeDataUri(await storage.cache.getOne(url)))
   ),
+});
+
+addPublicCommands({
   SetBadge: setBadge,
 });
 
diff --git a/src/background/utils/message.js b/src/background/utils/message.js
index 5ad7433d..a7cc00a1 100644
--- a/src/background/utils/message.js
+++ b/src/background/utils/message.js
@@ -1,6 +1,16 @@
 import { defaultImage, i18n, noop } from '@/common';
+import { forEachEntry } from '@/common/object';
 
 export const commands = {};
+export const addPublicCommands = Object.assign.bind(Object, commands);
+
+/** Commands that can be used only by an extension page i.e. not by a content script */
+export function addOwnCommands(obj) {
+  obj::forEachEntry(([name, fn]) => {
+    commands[name] = fn;
+    fn.isOwn = true;
+  });
+}
 
 export function notify(options) {
   browser.notifications.create(options.id || 'ViolentMonkey', {
diff --git a/src/background/utils/notifications.js b/src/background/utils/notifications.js
index d9846a1a..59e4e692 100644
--- a/src/background/utils/notifications.js
+++ b/src/background/utils/notifications.js
@@ -1,9 +1,9 @@
 import { i18n, defaultImage, sendTabCmd, trueJoin } from '@/common';
-import { commands } from './message';
+import { addPublicCommands } from './message';
 
 const openers = {};
 
-Object.assign(commands, {
+addPublicCommands({
   /** @return {Promise<string>} */
   async Notification({ image, text, title }, src, bgExtras) {
     const notificationId = await browser.notifications.create({
diff --git a/src/background/utils/options.js b/src/background/utils/options.js
index 4468dff4..55792410 100644
--- a/src/background/utils/options.js
+++ b/src/background/utils/options.js
@@ -2,14 +2,14 @@ import { debounce, ensureArray, initHooks, normalizeKeys } from '@/common';
 import { deepCopy, deepEqual, mapEntry, objectGet, objectSet } from '@/common/object';
 import defaults from '@/common/options-defaults';
 import { preInitialize } from './init';
-import { commands } from './message';
+import { addOwnCommands, commands } from './message';
 import storage from './storage';
 
 let changes;
 let initPending;
 let options = {};
 
-Object.assign(commands, {
+addOwnCommands({
   /** @return {Object} */
   GetAllOptions() {
     return commands.GetOptions(defaults);
diff --git a/src/background/utils/popup-tracker.js b/src/background/utils/popup-tracker.js
index 6648643a..01027b41 100644
--- a/src/background/utils/popup-tracker.js
+++ b/src/background/utils/popup-tracker.js
@@ -2,10 +2,18 @@ import { getActiveTab, sendTabCmd } from '@/common';
 import cache from './cache';
 import { getData } from './db';
 import { postInitialize } from './init';
-import { commands } from './message';
+import { addPublicCommands } from './message';
 
 export const popupTabs = {}; // { tabId: 1 }
 
+addPublicCommands({
+  async SetPopup(data, src) {
+    if (popupTabs[src.tab.id]) return;
+    Object.assign(data, await getData({ ids: data.ids }));
+    cache.put('SetPopup', Object.assign({ [src.frameId]: [data, src] }, cache.get('SetPopup')));
+  },
+});
+
 postInitialize.push(() => {
   browser.runtime.onConnect.addListener(onPopupOpened);
   browser.webRequest.onBeforeRequest.addListener(prefetchSetPopup, {
@@ -20,7 +28,6 @@ function onPopupOpened(port) {
   popupTabs[tabId] = 1;
   sendTabCmd(tabId, 'PopupShown', true);
   port.onDisconnect.addListener(onPopupClosed);
-  delete commands.SetPopup;
 }
 
 function onPopupClosed({ name }) {
@@ -31,8 +38,4 @@ function onPopupClosed({ name }) {
 async function prefetchSetPopup() {
   const tabId = (await getActiveTab()).id;
   sendTabCmd(tabId, 'PopupShown', true);
-  commands.SetPopup = async (data, src) => {
-    Object.assign(data, await getData({ ids: data.ids }));
-    cache.put('SetPopup', Object.assign({ [src.frameId]: [data, src] }, cache.get('SetPopup')));
-  };
 }
diff --git a/src/background/utils/preinject.js b/src/background/utils/preinject.js
index bf531c32..1987db22 100644
--- a/src/background/utils/preinject.js
+++ b/src/background/utils/preinject.js
@@ -8,7 +8,7 @@ import { forEachEntry, objectPick, objectSet } from '@/common/object';
 import ua from '@/common/ua';
 import { getScriptsByURL, ENV_CACHE_KEYS, ENV_REQ_KEYS, ENV_SCRIPTS, ENV_VALUE_IDS } from './db';
 import { postInitialize } from './init';
-import { commands } from './message';
+import { addPublicCommands } from './message';
 import { getOption, hookOptions } from './options';
 import { popupTabs } from './popup-tracker';
 import { clearRequestsByTabId } from './requests';
@@ -50,7 +50,7 @@ let isApplied;
 let injectInto;
 let xhrInject;
 
-Object.assign(commands, {
+addPublicCommands({
   /** @return {Promise<VMInjection>} */
   async GetInjected({ url, forceContent }, src) {
     const { frameId, tab } = src;
diff --git a/src/background/utils/requests.js b/src/background/utils/requests.js
index 80a3fd6c..3573580b 100644
--- a/src/background/utils/requests.js
+++ b/src/background/utils/requests.js
@@ -2,12 +2,12 @@ import { blob2base64, sendTabCmd, string2uint8array } from '@/common';
 import { forEachEntry, forEachValue, objectPick } from '@/common/object';
 import ua from '@/common/ua';
 import cache from './cache';
-import { commands } from './message';
+import { addPublicCommands, commands } from './message';
 import {
   FORBIDDEN_HEADER_RE, VM_VERIFY, requests, toggleHeaderInjector, verify,
 } from './requests-core';
 
-Object.assign(commands, {
+addPublicCommands({
   /**
    * @param {GMReq.Message.Web} opts
    * @param {MessageSender} src
diff --git a/src/background/utils/script.js b/src/background/utils/script.js
index b6331271..82a24e34 100644
--- a/src/background/utils/script.js
+++ b/src/background/utils/script.js
@@ -1,11 +1,11 @@
 import { getUniqId, encodeFilename } from '@/common';
 import { METABLOCK_RE } from '@/common/consts';
 import { mapEntry } from '@/common/object';
-import { commands } from './message';
+import { addOwnCommands } from './message';
 import { getOption } from './options';
 import cache from './cache';
 
-Object.assign(commands, {
+addOwnCommands({
   /** @return {string} */
   CacheNewScript(data) {
     const id = getUniqId();
diff --git a/src/background/utils/storage.js b/src/background/utils/storage.js
index f0e83126..1042e960 100644
--- a/src/background/utils/storage.js
+++ b/src/background/utils/storage.js
@@ -1,6 +1,6 @@
 import { mapEntry } from '@/common/object';
 import { ensureArray } from '@/common/util';
-import { commands } from './message';
+import { addOwnCommands } from './message';
 
 let api = browser.storage.local;
 
@@ -105,7 +105,7 @@ export const S_SCRIPT = storageByPrefix[S_SCRIPT_PRE].name;
 export const S_VALUE = storageByPrefix[S_VALUE_PRE].name;
 export default storage;
 
-Object.assign(commands, {
+addOwnCommands({
   Storage([area, method, ...args]) {
     return storage[area][method](...args);
   },
diff --git a/src/background/utils/tab-redirector.js b/src/background/utils/tab-redirector.js
index af5b4b68..3fc664ea 100644
--- a/src/background/utils/tab-redirector.js
+++ b/src/background/utils/tab-redirector.js
@@ -2,12 +2,12 @@ import { request, noop, i18n, getUniqId } from '@/common';
 import { extensionRoot } from '@/common/consts';
 import ua from '@/common/ua';
 import cache from './cache';
-import { commands } from './message';
+import { addPublicCommands, commands } from './message';
 import { parseMeta, isUserScript } from './script';
 
 const CONFIRM_URL_BASE = `${extensionRoot}confirm/index.html#`;
 
-Object.assign(commands, {
+addPublicCommands({
   async CheckInstallerTab(tabId, src) {
     const tab = IS_FIREFOX && (src.url || '').startsWith('file:')
       && await browser.tabs.get(tabId).catch(noop);
diff --git a/src/background/utils/tabs.js b/src/background/utils/tabs.js
index c2295d12..ed03c416 100644
--- a/src/background/utils/tabs.js
+++ b/src/background/utils/tabs.js
@@ -1,12 +1,12 @@
 import { getActiveTab, noop, sendTabCmd, getFullUrl } from '@/common';
 import { extensionRoot } from '@/common/consts';
 import ua from '@/common/ua';
-import { commands } from './message';
+import { addOwnCommands, addPublicCommands, commands } from './message';
 import { getOption } from './options';
 
 const openers = {};
 
-Object.assign(commands, {
+addOwnCommands({
   /**
    * @param {string} [pathId] - path or id to add to #scripts route in dashboard,
      if absent a new script will be created for active tab's URL
@@ -32,6 +32,9 @@ Object.assign(commands, {
     }
     return commands.TabOpen({ url, maybeInWindow: true }, src);
   },
+});
+
+addPublicCommands({
   /** @return {Promise<{ id: number } | chrome.tabs.Tab>} new tab is returned for internal calls */
   async TabOpen({
     url,
diff --git a/src/background/utils/update.js b/src/background/utils/update.js
index e50b31a1..4f5c6cbc 100644
--- a/src/background/utils/update.js
+++ b/src/background/utils/update.js
@@ -5,12 +5,12 @@ import {
 import { fetchResources, getScriptById, getScripts, parseScript } from './db';
 import { parseMeta } from './script';
 import { getOption, setOption } from './options';
-import { commands } from './message';
+import { addOwnCommands, commands } from './message';
 import { requestNewer } from './storage-fetch';
 
 const processes = {};
 
-Object.assign(commands, {
+addOwnCommands({
   /**
    * @param {number} [id] - when omitted, all scripts are checked
    * @return {Promise<number>} number of updated scripts
diff --git a/src/background/utils/values.js b/src/background/utils/values.js
index b45d2fb2..2c8ec2dd 100644
--- a/src/background/utils/values.js
+++ b/src/background/utils/values.js
@@ -1,7 +1,7 @@
 import { isEmpty, sendTabCmd } from '@/common';
 import { forEachEntry, objectGet, objectSet } from '@/common/object';
 import { getScript } from './db';
-import { commands } from './message';
+import { addOwnCommands, addPublicCommands } from './message';
 import storage from './storage';
 
 const nest = (obj, key) => obj[key] || (obj[key] = {}); // eslint-disable-line no-return-assign
@@ -10,7 +10,7 @@ const openers = {};
 let chain = Promise.resolve();
 let toSend = {};
 
-Object.assign(commands, {
+addOwnCommands({
   async GetValueStore(id, { tab }) {
     const frames = nest(nest(openers, id), tab.id);
     const values = frames[0] || (frames[0] = await storage.value.getOne(id));
@@ -32,6 +32,9 @@ Object.assign(commands, {
     commit(toWrite);
     return chain;
   },
+});
+
+addPublicCommands({
   /**
    * @return {?Promise<void>}
    */
diff --git a/src/common/browser.js b/src/common/browser.js
index 01398a80..2c556830 100644
--- a/src/common/browser.js
+++ b/src/common/browser.js
@@ -95,9 +95,9 @@ if (!IS_FIREFOX && !global.browser?.runtime) {
   // Both result and error must be explicitly specified to avoid prototype eavesdropping
   const wrapError = err => process.env.DEBUG && console.warn(err) || [
     null,
-    err instanceof SafeError
+    err?.[MESSAGE]
       ? [err[MESSAGE], err[STACK]]
-      : [err, ''],
+      : [err, new SafeError()[STACK]],
   ];
   const sendResponseAsync = async (result, sendResponse) => {
     try {
diff --git a/src/common/index.js b/src/common/index.js
index 22bc96b6..309b693e 100644
--- a/src/common/index.js
+++ b/src/common/index.js
@@ -80,9 +80,14 @@ const getBgPage = () => browser.extension.getBackgroundPage?.();
 export function sendCmdDirectly(cmd, data, options, fakeSrc) {
   const bg = !COMMANDS_WITH_SRC.includes(cmd) && getBgPage();
   const bgCopy = bg && bg !== window && bg.deepCopy;
-  return bgCopy
-    ? bg.handleCommandMessage(bgCopy({ cmd, data }), fakeSrc && bgCopy(fakeSrc)).then(deepCopy)
-    : sendCmd(cmd, data, options);
+  if (!bgCopy) {
+    return sendCmd(cmd, data, options);
+  }
+  if (fakeSrc) {
+    fakeSrc = bgCopy(fakeSrc);
+    fakeSrc.fake = true;
+  }
+  return bg.handleCommandMessage(bgCopy({ cmd, data }), fakeSrc).then(deepCopy);
 }
 
 /**


Test patch:
diff --git a/src/background/utils/tester.js b/src/background/utils/tester.js
index c273a396..514dab37 100644
--- a/src/background/utils/tester.js
+++ b/src/background/utils/tester.js
@@ -4,11 +4,11 @@ import { BLACKLIST, BLACKLIST_ERRORS } from '@/common/consts';
 import initCache from '@/common/cache';
 import * as tld from '@/common/tld';
 import { postInitialize } from './init';
-import { commands } from './message';
+import { addOwnCommands } from './message';
 import { getOption, hookOptions } from './options';
 import storage from './storage';
 
-Object.assign(commands, {
+addOwnCommands({
   TestBlacklist: testBlacklist,
 });