fix: don't use `common` in `injected` directly

.eslintrc.js

@@ -6,13 +6,11 @@ const FILES_CONTENT = [
   'src/injected/content/**/*.js',
 ];
 const FILES_WEB = [`src/injected/web/**/*.js`];
-  // some functions are used by `injected`
+/* Note that `injected` uses several more `common` files indirectly, but we check just these
+ * two automatically because they are trivial by design and must always pass the check */
 const FILES_SHARED = [
   'src/common/browser.js',
   'src/common/consts.js',
-  'src/common/index.js',
-  'src/common/object.js',
-  'src/common/util.js',
 ];
 
 const GLOBALS_COMMON = getGlobals('src/common/safe-globals.js');
@@ -66,6 +64,9 @@ module.exports = {
   }, {
     files: [...FILES_INJECTED, ...FILES_SHARED],
     rules: {
+      'no-restricted-imports': ['error', {
+        patterns: ['*/common', '*/common/*'],
+      }],
       /* Our .browserslistrc targets old browsers so the compiled code for {...objSpread} uses
          babel's polyfill that calls methods like `Object.assign` instead of our safe `assign`.
          Ideally, `eslint-plugin-compat` should be used but I couldn't make it work. */
@@ -82,6 +83,16 @@ module.exports = {
       }, {
         selector: ':matches(ArrayExpression, CallExpression) > SpreadElement',
         message: 'Spreading via Symbol.iterator may be spoofed/broken in an unsafe environment',
+      }, {
+        selector: '[callee.object.name="Object"], MemberExpression[object.name="Object"]',
+        message: 'Using potentially spoofed methods in an unsafe environment',
+        // TODO: auto-generate the rule using GLOBALS
+      }, {
+        selector: `CallExpression[callee.name="defineProperty"]:not(${[
+          '[arguments.2.properties.0.key.name="__proto__"]',
+          ':has(CallExpression[callee.name="createNullObj"])'
+        ].join(',')})`,
+        message: 'Prototype of descriptor may be spoofed/broken in an unsafe environment',
       }],
     },
   }, {

src/common/consts.js

@@ -1,4 +1,4 @@
-// SAFETY WARNING! Exports used by `injected` must make ::safe() calls
+// SAFETY WARNING! Exports used by `injected` must make ::safe() calls and use __proto__:null
 
 export const INJECT_AUTO = 'auto';
 export const INJECT_PAGE = 'page';

src/common/index.js

@@ -1,4 +1,4 @@
-// SAFETY WARNING! Exports used by `injected` must make ::safe() calls
+// SAFETY WARNING! Exports used by `injected` must make ::safe() calls and use __proto__:null
 
 import { browser } from '#/common/consts';
 import { deepCopy } from './object';
@@ -30,6 +30,7 @@ export function initHooks() {
 }
 
 /**
+ * Used by `injected`
  * @param {string} cmd
  * @param data
  * @param {{retry?: boolean, ignoreError?: boolean}} [options]
@@ -83,6 +84,7 @@ export function sendTabCmd(tabId, cmd, data, options) {
   return browser.tabs.sendMessage(tabId, { cmd, data }, options).catch(noop);
 }
 
+// Used by `injected`
 // ignoreError is always `true` when sending from the background script because it's a broadcast
 export function sendMessage(payload, { retry, ignoreError } = {}) {
   if (retry) return sendMessageRetry(payload);
@@ -94,6 +96,7 @@ export function sendMessage(payload, { retry, ignoreError } = {}) {
 }
 
 /**
+ * Used by `injected`
  * The active tab page and its [content] scripts load before the extension's
  * persistent background script when Chrome starts with a URL via command line
  * or when configured to restore the session, https://crbug.com/314686

src/common/object.js

@@ -1,12 +1,3 @@
-// SAFETY WARNING! Exports used by `injected` must make ::safe() calls
-
-const {
-  entries: objectEntries,
-  keys: objectKeys,
-  values: objectValues,
-} = Object;
-const { forEach, reduce } = [];
-
 export function normalizeKeys(key) {
   if (key == null) return [];
   if (Array.isArray(key)) return key;
@@ -51,7 +42,7 @@ export function objectSet(obj, rawKey, val) {
  * @returns {{}}
  */
 export function objectPick(obj, keys, transform) {
-  return keys::reduce((res, key) => {
+  return keys.reduce((res, key) => {
     let value = obj?.[key];
     if (transform) value = transform(value, key);
     if (value != null) res[key] = value;
@@ -61,7 +52,7 @@ export function objectPick(obj, keys, transform) {
 
 // invoked as obj::mapEntry(([key, value], i, allEntries) => transformedValue)
 export function mapEntry(func) {
-  return objectEntries(this)::reduce((res, entry, i, allEntries) => {
+  return Object.entries(this).reduce((res, entry, i, allEntries) => {
     res[entry[0]] = func(entry, i, allEntries);
     return res;
   }, {});
@@ -69,17 +60,17 @@ export function mapEntry(func) {
 
 // invoked as obj::forEachEntry(([key, value], i, allEntries) => {})
 export function forEachEntry(func) {
-  if (this) objectEntries(this)::forEach(func);
+  if (this) Object.entries(this).forEach(func);
 }
 
 // invoked as obj::forEachKey(key => {}, i, allKeys)
 export function forEachKey(func) {
-  if (this) objectKeys(this)::forEach(func);
+  if (this) Object.keys(this).forEach(func);
 }
 
 // invoked as obj::forEachValue(value => {}, i, allValues)
 export function forEachValue(func) {
-  if (this) objectValues(this)::forEach(func);
+  if (this) Object.values(this).forEach(func);
 }
 
 // Needed for Firefox's browser.storage API which fails on Vue observables

src/common/util.js

@@ -1,10 +1,7 @@
-// SAFETY WARNING! Exports used by `injected` must make ::safe() calls
+// SAFETY WARNING! Exports used by `injected` must make ::safe() calls and use __proto__:null
 
 import { browser } from '#/common/consts';
 
-export const isPromise = val => val::objectToString() === '[object Promise]';
-export const isFunction = val => typeof val === 'function';
-
 export function i18n(name, args) {
   return browser.i18n.getMessage(name, args) || name;
 }
@@ -195,6 +192,7 @@ export const formatByteLength = len => (
     || `${+(len / (1024 * 1024)).toFixed(1)} M` // allow fractions for megabytes
 );
 
+// Used by `injected`
 export function isEmpty(obj) {
   for (const key in obj) {
     if (obj::hasOwnProperty(key)) {
@@ -305,6 +303,7 @@ export async function request(url, options = {}) {
   return result;
 }
 
+// Used by `injected`
 const SIMPLE_VALUE_TYPE = {
   __proto__: null,
   string: 's',
@@ -312,6 +311,7 @@ const SIMPLE_VALUE_TYPE = {
   boolean: 'b',
 };
 
+// Used by `injected`
 export function dumpScriptValue(value, jsonDump = JSON.stringify) {
   if (value !== undefined) {
     const simple = SIMPLE_VALUE_TYPE[typeof value];

src/injected/content/bridge.js

@@ -1,5 +1,4 @@
-import { isPromise, sendCmd } from '#/common';
-import { INJECT_PAGE, browser } from '#/common/consts';
+import { INJECT_PAGE, browser, sendCmd } from '../util';
 
 const allowed = createNullObj();
 const dataKeyNameMap = createNullObj();

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

@@ -1,4 +1,4 @@
-import { sendCmd } from '#/common';
+import { sendCmd } from '../util';
 import bridge from './bridge';
 import { decodeResource, elemByTag, makeElem } from './util-content';
 

src/injected/content/index.js

@@ -1,5 +1,4 @@
-import { isEmpty, sendCmd } from '#/common';
-import { INJECT_CONTENT } from '#/common/consts';
+import { isEmpty, sendCmd, INJECT_CONTENT } from '../util';
 import bridge from './bridge';
 import './clipboard';
 import { sendSetPopup } from './gm-api-content';

src/injected/content/inject.js

@@ -1,9 +1,9 @@
-import { INJECT_CONTENT, INJECT_MAPPING, INJECT_PAGE, browser } from '#/common/consts';
-import { sendCmd } from '#/common';
-import { forEachKey } from '#/common/object';
 import bridge from './bridge';
 import { appendToRoot, onElement } from './util-content';
-import { bindEvents, fireBridgeEvent } from '../util';
+import {
+  bindEvents, fireBridgeEvent,
+  INJECT_CONTENT, INJECT_MAPPING, INJECT_PAGE, browser, sendCmd,
+} from '../util';
 
 const INIT_FUNC_NAME = process.env.INIT_FUNC_NAME;
 const VAULT_SEED_NAME = INIT_FUNC_NAME + process.env.VAULT_ID_NAME;
@@ -209,7 +209,7 @@ function inject(item) {
 }
 
 function injectAll(runAt) {
-  realms::forEachKey((realm) => {
+  for (const realm in realms) { /* proto is null */// eslint-disable-line guard-for-in
     const realmData = realms[realm];
     const items = realmData.lists[runAt];
     const { info } = realmData;
@@ -219,7 +219,7 @@ function injectAll(runAt) {
         injectList(runAt);
       }
     }
-  });
+  }
   if (runAt !== 'start' && contLists[runAt].length) {
     bridge.post('RunAt', runAt, INJECT_CONTENT);
   }

src/injected/content/notifications.js

@@ -1,4 +1,4 @@
-import { sendCmd } from '#/common';
+import { sendCmd } from '../util';
 import bridge from './bridge';
 
 const notifications = createNullObj();

src/injected/content/requests.js

@@ -1,4 +1,4 @@
-import { isPromise, sendCmd } from '#/common';
+import { sendCmd } from '../util';
 import bridge from './bridge';
 import { getFullUrl, makeElem } from './util-content';
 

src/injected/content/tabs.js

@@ -1,4 +1,4 @@
-import { sendCmd } from '#/common';
+import { sendCmd } from '../util';
 import bridge from './bridge';
 
 const tabIds = createNullObj();

src/injected/index.js

@@ -1,5 +1,5 @@
-import '#/common/browser';
-import { sendCmd } from '#/common';
+import '#/common/browser'; // eslint-disable-line no-restricted-imports
+import { sendCmd } from './util';
 import './content';
 
 // Script installation in Firefox as it does not support `onBeforeRequest` for `file:`

src/injected/safe-globals-injected.js

@@ -4,6 +4,7 @@
  * This file is used by both `injected` and `injected-web` entries.
  * `global` is used instead of WebPack's polyfill which we disable in webpack.conf.js.
  * `export` is stripped in the final output and is only used for our NodeJS test scripts.
+ * WARNING! Don't use exported functions from #/common anywhere in injected!
  */
 
 const global = (function _() {
@@ -19,6 +20,10 @@ export const WINDOW_FOCUS = 'window.focus';
 export const NS_HTML = 'http://www.w3.org/1999/xhtml';
 export const CALLBACK_ID = '__CBID';
 
+export const isPromise = val => val::objectToString() === '[object Promise]';
+
+export const isFunction = val => typeof val === 'function';
+
 export const getOwnProp = (obj, key) => (
   obj::hasOwnProperty(key)
     ? obj[key]

src/injected/util/index.js

@@ -1,3 +1,17 @@
+/* eslint-disable no-restricted-imports */
+
+/* WARNING!
+ * Make sure all re-exported functions survive in a spoofed/broken environment:
+ * use only ::safe() globals that are initialized in a corresponding safe-globals* file,
+ * use __proto__:null or get/set own props explicitly. */
+
+export {
+  dumpScriptValue,
+  isEmpty,
+  sendCmd,
+} from '#/common';
+export * from '#/common/consts';
+
 export const fireBridgeEvent = (eventId, msg, cloneInto) => {
   const detail = cloneInto ? cloneInto(msg, document) : msg;
   const evtMain = new CustomEventSafe(eventId, { detail });

src/injected/web/gm-api.js

@@ -1,4 +1,4 @@
-import { dumpScriptValue, isEmpty, isFunction } from '#/common/util';
+import { dumpScriptValue, isEmpty } from '../util';
 import bridge from './bridge';
 import store from './store';
 import { onTabCreate } from './tabs';

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

@@ -1,5 +1,4 @@
-import { isFunction } from '#/common';
-import { INJECT_CONTENT } from '#/common/consts';
+import { INJECT_CONTENT } from '../util';
 import bridge from './bridge';
 import { FastLookup } from './util-web';
 

src/injected/web/gm-values.js

@@ -1,4 +1,3 @@
-import { forEachEntry } from '#/common/object';
 import bridge from './bridge';
 import store from './store';
 
@@ -15,11 +14,10 @@ const dataDecoders = {
 bridge.addHandlers({
   UpdatedValues(updates) {
     const { partial } = updates;
-    updates::forEachEntry(entry => {
-      const id = entry[0];
-      const update = entry[1];
+    objectKeys(updates)::forEach(id => {
       const oldData = store.values[id];
       if (oldData) {
+        const update = updates[id];
         const keyHooks = changeHooks[id];
         if (keyHooks) changedRemotely(keyHooks, oldData, update);
         if (partial) applyPartialUpdate(oldData, update);
@@ -54,20 +52,18 @@ export function decodeValue(raw) {
 }
 
 function applyPartialUpdate(data, update) {
-  update::forEachEntry(entry => {
-    const key = entry[0];
-    const val = entry[1];
+  objectKeys(update)::forEach(key => {
+    const val = update[key];
     if (val) data[key] = val;
     else delete data[key];
   });
 }
 
 function changedRemotely(keyHooks, data, update) {
-  update::forEachEntry(entry => {
-    const key = entry[0];
+  objectKeys(update)::forEach(key => {
     const hooks = keyHooks[key];
     if (hooks) {
-      let raw = entry[1];
+      let raw = update[key];
       if (!raw) raw = undefined; // partial `update` currently uses null for deleted values
       const oldRaw = data[key];
       if (oldRaw !== raw) {

src/injected/web/index.js

@@ -1,4 +1,3 @@
-import { INJECT_PAGE, INJECT_CONTENT } from '#/common/consts';
 import bridge from './bridge';
 import store from './store';
 import { makeGmApiWrapper } from './gm-api-wrapper';
@@ -6,7 +5,7 @@ import './gm-values';
 import './notifications';
 import './requests';
 import './tabs';
-import { bindEvents } from '../util';
+import { bindEvents, INJECT_PAGE, INJECT_CONTENT } from '../util';
 
 // Make sure to call safe::methods() in code that may run after userscripts
 

src/injected/web/requests.js

@@ -1,4 +1,3 @@
-import { isFunction } from '#/common';
 import bridge from './bridge';
 
 const idMap = createNullObj();
@@ -66,7 +65,7 @@ function callback(req, msg) {
       __proto__: null,
       get() {
         const value = 'raw' in req ? parseData(req, msg) : req.response;
-        defineProperty(this, 'response', { value, __proto__: null });
+        defineProperty(this, 'response', { __proto__: null, value });
         return value;
       },
     });

src/injected/web/util-web.js

@@ -1,4 +1,4 @@
-import { INJECT_CONTENT } from '#/common/consts';
+import { INJECT_CONTENT } from '../util';
 import bridge from './bridge';
 
 // Firefox defines `isFinite` on `global` not on `window`