Browse Source

fix: nuke proto on vulnerable objects, #1400

+ also disable array destructuring in injected as it can be easily broken by overriding Symbol.iterator
tophf 4 years ago
parent
commit
40f3bff8eb

+ 3 - 0
.eslintrc.js

@@ -53,6 +53,9 @@ module.exports = {
       }, {
         selector: 'OptionalCallExpression',
         message: 'Optional call in an unsafe environment',
+      }, {
+        selector: 'ArrayPattern',
+        message: 'Array destructuring in an unsafe environment',
       }],
     },
   }],

+ 2 - 2
src/background/utils/preinject.js

@@ -46,7 +46,7 @@ Object.assign(commands, {
         env.forceContent = !pageInjectable;
         scripts.map(prepareScript, env).filter(Boolean).forEach(processFeedback, src);
         return {
-          info: { cache: env.cache },
+          cache: env.cache,
           scripts,
         };
       }
@@ -242,7 +242,7 @@ function prepareScript(script) {
     code: isContent ? '' : forceContent || injectedCode,
     injectInto: realm,
     metaStr: code.match(METABLOCK_RE)[1] || '',
-    values: value[id],
+    values: value[id] || null,
   });
   return isContent && [dataKey, true];
 }

+ 4 - 2
src/injected/content/bridge.js

@@ -1,10 +1,12 @@
 import { sendCmd } from '#/common';
 import { INJECT_PAGE, browser } from '#/common/consts';
 
+// {CommandName: sendCmd} will relay the request via sendCmd as is
 /** @type {Object.<string, MessageFromGuestHandler>} */
-const handlers = {};
-const bgHandlers = {};
+const handlers = createNullObj();
+const bgHandlers = createNullObj();
 const bridge = {
+  __proto__: null, // Object.create(null) may be spoofed
   ids: [], // all ids including the disabled ones for SetPopup
   runningIds: [],
   // userscripts running in the content script context are messaged via invokeGuest

+ 1 - 0
src/injected/content/clipboard.js

@@ -11,6 +11,7 @@ const { preventDefault, stopImmediatePropagation } = Event.prototype;
 let clipboardData;
 
 bridge.addHandlers({
+  __proto__: null, // Object.create(null) may be spoofed
   SetClipboard(data) {
     if (bridge.isFirefox) {
       // Firefox does not support copy from background page.

+ 10 - 7
src/injected/content/index.js

@@ -52,9 +52,9 @@ const { split } = '';
 })().catch(IS_FIREFOX && console.error); // Firefox can't show exceptions in content scripts
 
 bridge.addBackgroundHandlers({
+  __proto__: null, // Object.create(null) may be spoofed
   Command(data) {
-    const [cmd] = data;
-    const id = +cmd::split(':', 1)[0];
+    const id = +data[0]::split(':', 1)[0];
     const realm = invokableIds::includes(id) && INJECT_CONTENT;
     bridge.post('Command', data, realm);
   },
@@ -63,8 +63,8 @@ bridge.addBackgroundHandlers({
     sendSetPopup();
   },
   UpdatedValues(data) {
-    const dataPage = {};
-    const dataContent = {};
+    const dataPage = createNullObj();
+    const dataContent = createNullObj();
     objectKeys(data)::forEach((id) => {
       (invokableIds::includes(+id) ? dataContent : dataPage)[id] = data[id];
     });
@@ -74,10 +74,12 @@ bridge.addBackgroundHandlers({
 });
 
 bridge.addHandlers({
+  __proto__: null, // Object.create(null) may be spoofed
   UpdateValue: sendCmd,
   RegisterMenu(data) {
     if (IS_TOP) {
-      const [id, cap] = data;
+      const id = data[0];
+      const cap = data[1];
       const commandMap = menus[id] || (menus[id] = {});
       commandMap[cap] = 1;
       sendSetPopup(true);
@@ -85,12 +87,13 @@ bridge.addHandlers({
   },
   UnregisterMenu(data) {
     if (IS_TOP) {
-      const [id, cap] = data;
+      const id = data[0];
+      const cap = data[1];
       delete menus[id]?.[cap];
       sendSetPopup(true);
     }
   },
-  AddElement([tag, attributes, id]) {
+  AddElement({ tag, attributes, id }) {
     try {
       const el = document::createElementNS(NS_HTML, tag);
       el::setAttribute('id', id);

+ 7 - 6
src/injected/content/inject.js

@@ -24,6 +24,7 @@ let numBadgesSent = 0;
 let bfCacheWired;
 
 bridge.addHandlers({
+  __proto__: null, // Object.create(null) may be spoofed
   // FF bug workaround to enable processing of sourceURL in injected page scripts
   InjectList: injectList,
   Run(id, realm) {
@@ -80,16 +81,19 @@ export async function injectScripts(data, isXml) {
   const { hasMore, info } = data;
   pageInjectable = isXml ? false : null;
   realms = {
+    __proto__: null, // Object.create(null) may be spoofed
     /** @namespace VMInjectionRealm */
     [INJECT_CONTENT]: {
       injectable: () => true,
       /** @namespace VMRunAtLists */
       lists: contLists = { start: [], body: [], end: [], idle: [] },
+      is: 0,
       info,
     },
     [INJECT_PAGE]: {
       injectable: () => pageInjectable ?? checkInjectable(),
       lists: pgLists = { start: [], body: [], end: [], idle: [] },
+      is: 0,
       info,
     },
   };
@@ -137,9 +141,9 @@ export async function injectScripts(data, isXml) {
   });
 }
 
-async function injectDelayedScripts({ info, scripts }, getReadyState, hasInvoker) {
+async function injectDelayedScripts({ cache, scripts }, getReadyState, hasInvoker) {
   realms::forEachKey(r => {
-    assign(realms[r].info, info);
+    realms[r].info.cache = cache;
   });
   let needsInvoker;
   scripts::forEach(script => {
@@ -241,14 +245,11 @@ function onElement(tag, cb, ...args) {
     } else {
       const observer = new MutationObserver(() => {
         if (elemByTag(tag)) {
-          observer::disconnect(); // eslint-disable-line no-use-before-define
+          observer.disconnect();
           cb(...args);
           resolve();
         }
       });
-      // This function starts before any content-mode userscripts, but observer's callback
-      // will run after document-start content-mode userscripts, so we'll use safe calls.
-      const { disconnect } = observer;
       // documentElement may be replaced so we'll observe the entire document
       observer.observe(document, { childList: true, subtree: true });
     }

+ 9 - 7
src/injected/content/notifications.js

@@ -1,15 +1,16 @@
 import { sendCmd } from '#/common';
 import bridge from './bridge';
 
-const notifications = {};
+const notifications = createNullObj();
 
 bridge.addHandlers({
+  __proto__: null, // Object.create(null) may be spoofed
   async Notification(options, realm) {
     const nid = await sendCmd('Notification', options);
     notifications[nid] = { id: options.id, realm };
   },
   RemoveNotification(id) {
-    const nid = objectEntries(notifications)::filter(([, val]) => val.id === id)[0]?.[0];
+    const nid = objectEntries(notifications).find(entry => entry[1].id === id)?.[0];
     if (nid) {
       delete notifications[nid];
       return sendCmd('RemoveNotification', nid);
@@ -18,14 +19,15 @@ bridge.addHandlers({
 });
 
 bridge.addBackgroundHandlers({
+  __proto__: null, // Object.create(null) may be spoofed
   NotificationClick(nid) {
-    const { id, realm } = notifications[nid] || {};
-    if (id) bridge.post('NotificationClicked', id, realm);
+    const n = notifications[nid];
+    if (n) bridge.post('NotificationClicked', n.id, n.realm);
   },
   NotificationClose(nid) {
-    const { id, realm } = notifications[nid] || {};
-    if (id) {
-      bridge.post('NotificationClosed', id, realm);
+    const n = notifications[nid];
+    if (n) {
+      bridge.post('NotificationClosed', n.id, n.realm);
       delete notifications[nid];
     }
   },

+ 3 - 1
src/injected/content/requests.js

@@ -4,9 +4,10 @@ import bridge from './bridge';
 const { fetch } = global;
 const { arrayBuffer: getArrayBuffer, blob: getBlob } = Response.prototype;
 
-const requests = {};
+const requests = createNullObj();
 
 bridge.addHandlers({
+  __proto__: null, // Object.create(null) may be spoofed
   HttpRequest(opts, realm) {
     requests[opts.id] = {
       realm,
@@ -19,6 +20,7 @@ bridge.addHandlers({
 });
 
 bridge.addBackgroundHandlers({
+  __proto__: null, // Object.create(null) may be spoofed
   async HttpRequested(msg) {
     const { blobbed, id, numChunks, type } = msg;
     const req = requests[id];

+ 2 - 0
src/injected/content/tabs.js

@@ -6,6 +6,7 @@ const tabKeys = {};
 const realms = {};
 
 bridge.addHandlers({
+  __proto__: null, // Object.create(null) may be spoofed
   async TabOpen({ key, data }, realm) {
     const { id } = await sendCmd('TabOpen', data);
     tabIds[key] = id;
@@ -21,6 +22,7 @@ bridge.addHandlers({
 });
 
 bridge.addBackgroundHandlers({
+  __proto__: null, // Object.create(null) may be spoofed
   TabClosed(id) {
     const key = tabKeys[id];
     const realm = realms[id];

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

@@ -17,3 +17,5 @@ export const {
   values: objectValues,
 } = Object;
 export const { parse: jsonParse } = JSON;
+// Using __proto__ because Object.create(null) may be spoofed
+export const createNullObj = () => ({ __proto__: null });

+ 4 - 3
src/injected/web/bridge.js

@@ -1,9 +1,10 @@
 import { getUniqId } from '#/common';
 
-const handlers = {};
-const callbacks = {};
+const handlers = createNullObj();
+const callbacks = createNullObj();
 const bridge = {
-  cache: {},
+  __proto__: null, // Object.create(null) may be spoofed
+  cache: createNullObj(),
   callbacks,
   addHandlers(obj) {
     assign(handlers, obj);

+ 8 - 20
src/injected/web/gm-api.js

@@ -27,7 +27,7 @@ export const vmOwnFunc = (func, toString) => {
 let downloadChain = Promise.resolve();
 
 export function makeGmApi() {
-  return [{
+  return {
     GM_deleteValue(key) {
       const { id } = this;
       const values = loadValues(id);
@@ -66,8 +66,8 @@ export function makeGmApi() {
     GM_addValueChangeListener(key, fn) {
       if (typeof key !== 'string') key = `${key}`;
       if (typeof fn !== 'function') return;
-      const keyHooks = changeHooks[this.id] || (changeHooks[this.id] = {});
-      const hooks = keyHooks[key] || (keyHooks[key] = {});
+      const keyHooks = changeHooks[this.id] || (changeHooks[this.id] = createNullObj());
+      const hooks = keyHooks[key] || (keyHooks[key] = createNullObj());
       const i = objectValues(hooks)::indexOf(fn);
       let listenerId = i >= 0 && objectKeys(hooks)[i];
       if (!listenerId) {
@@ -82,7 +82,9 @@ export function makeGmApi() {
     GM_removeValueChangeListener(listenerId) {
       const keyHooks = changeHooks[this.id];
       if (!keyHooks) return;
-      objectEntries(keyHooks)::findIndex(([key, hooks]) => {
+      objectEntries(keyHooks)::findIndex(keyHook => {
+        const key = keyHook[0];
+        const hooks = keyHook[1];
         if (listenerId in hooks) {
           delete hooks[listenerId];
           if (isEmpty(hooks)) delete keyHooks[key];
@@ -188,28 +190,14 @@ export function makeGmApi() {
     },
     // using the native console.log so the output has a clickable link to the caller's source
     GM_log: logging.log,
-  }, {
-    // Greasemonkey4 API polyfill
-    getResourceURL: { async: true },
-    getValue: { async: true },
-    deleteValue: { async: true },
-    setValue: { async: true },
-    listValues: { async: true },
-    xmlHttpRequest: { alias: 'xmlhttpRequest' },
-    notification: true,
-    openInTab: true,
-    registerMenuCommand: true,
-    setClipboard: true,
-    addStyle: true, // gm4-polyfill.js sets it anyway
-    addElement: true, // TM sets it
-  }];
+  };
 }
 
 function webAddElement(parent, tag, attributes, useId) {
   const id = useId || getUniqId('VMel');
   let el;
   // DOM error in content script can't be caught by a page-mode userscript so we rethrow it here
-  let error = bridge.sendSync('AddElement', [tag, attributes, id]);
+  let error = bridge.sendSync('AddElement', { tag, attributes, id });
   if (!error) {
     try {
       el = document::getElementById(id);

+ 18 - 13
src/injected/web/gm-values.js

@@ -6,7 +6,7 @@ import { log } from '../utils/helpers';
 const { Number } = global;
 
 // Nested objects: scriptId -> keyName -> listenerId -> GMValueChangeListener
-export const changeHooks = {};
+export const changeHooks = createNullObj();
 
 const dataDecoders = {
   o: jsonParse,
@@ -15,9 +15,12 @@ const dataDecoders = {
 };
 
 bridge.addHandlers({
+  __proto__: null, // Object.create(null) may be spoofed
   UpdatedValues(updates) {
     const { partial } = updates;
-    updates::forEachEntry(([id, update]) => {
+    updates::forEachEntry(entry => {
+      const id = entry[0];
+      const update = entry[1];
       const oldData = store.values[id];
       if (oldData) {
         const keyHooks = changeHooks[id];
@@ -54,16 +57,20 @@ export function decodeValue(raw) {
 }
 
 function applyPartialUpdate(data, update) {
-  update::forEachEntry(([key, val]) => {
+  update::forEachEntry(entry => {
+    const key = entry[0];
+    const val = entry[1];
     if (val) data[key] = val;
     else delete data[key];
   });
 }
 
 function changedRemotely(keyHooks, data, update) {
-  update::forEachEntry(([key, raw]) => {
+  update::forEachEntry(entry => {
+    const key = entry[0];
     const hooks = keyHooks[key];
     if (hooks) {
+      let raw = entry[1];
       if (!raw) raw = undefined; // partial `update` currently uses null for deleted values
       const oldRaw = data[key];
       if (oldRaw !== raw) {
@@ -78,13 +85,11 @@ function notifyChange(hooks, key, val, raw, oldRaw, remote = false) {
   // converting `null` from messaging to `undefined` to match the documentation and TM
   const oldVal = (oldRaw || undefined) && decodeValue(oldRaw);
   const newVal = val === undefined && raw ? decodeValue(raw) : val;
-  objectValues(hooks)::forEach(fn => tryCall(fn, key, oldVal, newVal, remote));
-}
-
-function tryCall(fn, ...args) {
-  try {
-    fn(...args);
-  } catch (e) {
-    log('error', ['GM_addValueChangeListener', 'callback'], e);
-  }
+  objectValues(hooks)::forEach(fn => {
+    try {
+      fn(key, oldVal, newVal, remote);
+    } catch (e) {
+      log('error', ['GM_addValueChangeListener', 'callback'], e);
+    }
+  });
 }

+ 29 - 14
src/injected/web/gm-wrapper.js

@@ -12,12 +12,21 @@ const {
 } = global;
 /** A bound function won't be stepped-into when debugging.
  * Destructuring a random function reference instead of the long `Function.prototype` */
-const { bind } = Proxy;
+const { apply, bind } = Proxy;
 const { concat, slice: arraySlice } = [];
 const { startsWith } = '';
 
+// Greasemonkey4 API polyfill exceptions for async (value=0) and alias (value='string')
+const gm4Api = {
+  __proto__: null, // Object.create(null) may be spoofed
+  getResourceURL: 0,
+  getValue: 0,
+  deleteValue: 0,
+  setValue: 0,
+  listValues: 0,
+  xmlHttpRequest: 'xmlhttpRequest',
+};
 let gmApi;
-let gm4Api;
 let componentUtils;
 const IS_TOP = window.top === window;
 
@@ -29,17 +38,21 @@ export function wrapGM(script) {
     grant.length = 0;
   }
   const id = script.props.id;
-  const resources = script.meta.resources || {};
+  const resources = script.meta.resources || createNullObj();
   const context = {
     id,
     script,
     resources,
-    pathMap: script.custom.pathMap || {},
-    urls: {},
+    pathMap: script.custom.pathMap || createNullObj(),
+    urls: createNullObj(),
   };
   const gmInfo = makeGmInfo(script, resources);
   const gm = {
-    GM: { info: gmInfo },
+    __proto__: null, // Object.create(null) may be spoofed
+    GM: {
+      __proto__: null,
+      info: gmInfo,
+    },
     GM_info: gmInfo,
     unsafeWindow: global,
   };
@@ -54,14 +67,14 @@ export function wrapGM(script) {
   if (grant::includes('window.focus')) {
     gm.focus = vmOwnFunc(() => bridge.post('TabFocus'));
   }
-  if (!gmApi) [gmApi, gm4Api] = makeGmApi();
+  if (!gmApi) gmApi = makeGmApi();
   grant::forEach((name) => {
     const gm4name = name::startsWith('GM.') && name::slice(3);
     const gm4 = gm4Api[gm4name];
-    const method = gmApi[gm4 ? `GM_${gm4.alias || gm4name}` : name];
+    const method = gmApi[gm4name ? `GM_${gm4 || gm4name}` : name];
     if (method) {
-      const caller = makeGmMethodCaller(method, context, gm4?.async);
-      if (gm4) gm.GM[gm4name] = caller;
+      const caller = makeGmMethodCaller(method, context, gm4 === 0);
+      if (gm4name) gm.GM[gm4name] = caller;
       else gm[name] = caller;
     }
   });
@@ -117,7 +130,7 @@ function makeGmMethodCaller(gmMethod, context, isAsync) {
   // keeping the native console.log intact
   return gmMethod === gmApi.GM_log ? gmMethod : vmOwnFunc(
     isAsync
-      ? (async (...args) => context::gmMethod(...args))
+      ? (async (...args) => gmMethod::apply(context, args))
       : gmMethod::bind(context),
   );
 }
@@ -256,7 +269,7 @@ boundMethods.get = mapGet;
  * @desc Wrap helpers to prevent unexpected modifications.
  */
 function makeGlobalWrapper(local) {
-  const events = {};
+  const events = createNullObj();
   const scopeSym = Symbol.unscopables;
   const globals = new Set(globalKeys);
   globals[iterSym] = setIter;
@@ -328,14 +341,16 @@ function makeGlobalWrapper(local) {
       return true;
     },
   });
-  for (const [name, desc] of unforgeables) {
+  unforgeables::forEach(entry => {
+    const name = entry[0];
+    const desc = entry[1];
     if (name === 'window' || name === 'top' && IS_TOP) {
       delete desc.get;
       delete desc.set;
       desc.value = wrapper;
     }
     defineProperty(local, name, mapWindow(desc));
-  }
+  });
   function mapWindow(desc) {
     if (desc && desc.value === window) {
       desc = assign({}, desc);

+ 10 - 13
src/injected/web/index.js

@@ -15,8 +15,8 @@ const { KeyboardEvent, MouseEvent } = global;
 const { get: getCurrentScript } = describeProperty(Document.prototype, 'currentScript');
 
 const sendSetTimeout = () => bridge.send('SetTimeout', 0);
-const resolvers = {};
-const waiters = {};
+const resolvers = createNullObj();
+const waiters = createNullObj();
 
 export default function initialize(
   webId,
@@ -46,7 +46,10 @@ export default function initialize(
 }
 
 bridge.addHandlers({
-  Command([cmd, evt]) {
+  __proto__: null, // Object.create(null) may be spoofed
+  Command(data) {
+    const cmd = data[0];
+    const evt = data[1];
     const constructor = evt.key ? KeyboardEvent : MouseEvent;
     const fn = store.commands[cmd];
     if (fn) fn(new constructor(evt.type, evt));
@@ -71,19 +74,13 @@ bridge.addHandlers({
     }
   },
   Expose() {
-    const Violentmonkey = {};
-    defineProperty(Violentmonkey, 'version', {
-      value: process.env.VM_VER,
-    });
-    defineProperty(Violentmonkey, 'isInstalled', {
-      async value(name, namespace) {
+    window.external.Violentmonkey = {
+      version: process.env.VM_VER,
+      async isInstalled(name, namespace) {
         const script = await bridge.send('GetScript', { meta: { name, namespace } });
         return script && !script.config.removed ? script.meta.version : null;
       },
-    });
-    defineProperty(window.external, 'Violentmonkey', {
-      value: Violentmonkey,
-    });
+    };
   },
 });
 

+ 1 - 0
src/injected/web/notifications.js

@@ -4,6 +4,7 @@ let lastId = 0;
 const notifications = {};
 
 bridge.addHandlers({
+  __proto__: null, // Object.create(null) may be spoofed
   NotificationClicked(id) {
     const fn = notifications[id]?.onclick;
     if (fn) fn();

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

@@ -13,6 +13,7 @@ const { get: getHref } = describeProperty(HTMLAnchorElement.prototype, 'href');
 const { readAsDataURL } = FileReader.prototype;
 
 bridge.addHandlers({
+  __proto__: null, // Object.create(null) may be spoofed
   HttpRequested(msg) {
     const req = idMap[msg.id];
     if (req) callback(req, msg);
@@ -30,7 +31,6 @@ export function onRequestCreate(opts, scriptId) {
   start(req);
   return {
     abort() {
-      req._aborted = true;
       bridge.post('AbortRequest', id);
     },
   };

+ 3 - 2
src/injected/web/store.js

@@ -1,5 +1,6 @@
 export default {
-  commands: {},
-  values: {},
+  __proto__: null, // Object.create(null) may be spoofed
+  commands: createNullObj(),
+  values: createNullObj(),
   state: 0,
 };

+ 2 - 1
src/injected/web/tabs.js

@@ -1,9 +1,10 @@
 import bridge from './bridge';
 
 let lastId = 0;
-const tabs = {};
+const tabs = createNullObj();
 
 bridge.addHandlers({
+  __proto__: null, // Object.create(null) may be spoofed
   TabClosed(key) {
     const item = tabs[key];
     if (item) {