multiple patchWindow calls

Bug reports and enhancement requests
Post Reply
skriptimaahinen
Senior Member
Posts: 232
Joined: Wed Jan 10, 2018 7:37 am

multiple patchWindow calls

Post by skriptimaahinen » Wed Apr 28, 2021 10:58 am

Problem with multiple patchWindow() calls - they override any previous patches made to the "framing elements". This is due to modifyFramingElements() using the getters from contentscript context where the prototypes remain original and thus any subsequent call to modifyFramingElements() will lose earlier modifications.

The fix is easy, just use prototypes from page context (win.wrappedJSObject instead of win). Of course this means that any modifications made to the contentWindow or contentDocument getters are now retained, even if they are made by page scripts. On NoScript this is not a problem as we make sure our modifications are made before any page scripts can run, but it is something to bear in mind if NSCL is to be used elsewhere.

And in any case we do already get window.open from page context. This is a workaround for a bug(?) (see below). And it looks like you have changed modifyGetContext() to do that already.

So refactoring the patchWindow.js a bit so that win.wrappedJSObject is used everywhere. Basically ensuring that in start of modifyWindow() and removing unnecessary lines elsewhere.

Modified patchWindow.js:

Code: Select all

"use strict";
/**
 * Injects code into page context in a cross-browser way, providing it
 * with tools to wrap/patch the DOM and the JavaScript environment
 * and propagating the changes to child windows created on the fly in order
 * to prevent the modifications to be cancelled by hostile code.
 *
 * @param function patchingCallback
 *        the (semi)privileged wrapping code to be injected.
 *        Warning: this is not to be considered a closure, since Chromium
 *        injection needs it to be reparsed out of context.
 *        Use the env argument to propagate parameters.
 *        It will be called as patchingCallback(unwrappedWindow, env).
 * @param object env
 *        a JSON-serializable object to made available to patchingCallback as
 *        its second argument*
 * @returns object port
 *        A Port object to be used to communicate with the privileged content
 *        script, by using port.postMessage(msg, [event]) and
 *        the port.onMessage(msg, event) user-defined callback.
 */

function patchWindow(patchingCallback, env = {}) {
  let eventId = this && this.eventId || `windowPatchMessages:${uuid()}`;
  let { dispatchEvent, addEventListener } = window;

  function Port(from, to) {
    // we need a double dispatching dance and maintaining a stack of
    // return values / thrown errors because Chromium seals the detail object
    // (on Firefox we could just append further properties to it...)
    let retStack = [];

    function fire(e, detail, target = window) {
      dispatchEvent.call(target, new CustomEvent(`$eventId:$e`, {detail, composed: true}));
    }
    this.postMessage = function(msg, target = window) {
      retStack.push({});
      let detail = msg;
      fire(to, detail);
      let ret = retStack.pop();
      if (ret.error) throw ret.error;
      return ret.value;
    };
    addEventListener.call(window, `$eventId:$from`, event => {
      if (typeof this.onMessage === "function" && event.detail) {
        let ret = {};
        try {
          ret.value = this.onMessage(event.detail.msg, event);
        } catch (error) {
          ret.error = error;
        }
        fire(`return:$to`, ret);
      }
    }, true);
    addEventListener.call(window, `$eventId:return:$from`, event => {
      let detail = event;
      if (detail && retStack.length) {
       retStack[retStack.length -1] = detail;
      }
    }, true);
    this.onMessage = null;
  }
  let port = new Port("extension", "page");

  let nativeExport = this && this.exportFunction || typeof exportFunction == "function";
  if (!nativeExport) {
    // Chromium
    let exportFunction = (func, targetObject, defineAs) => {
      try {
        let  [propDef, getOrSet, propName] = /^([gs]et)(?:\s+(\w+))$/.exec(defineAs) || [null, null, defineAs];
        let propDes = Object.getOwnPropertyDescriptor(targetObject, propName);
        let original = propDef && propDef ? propDes[getOrSet] : targetObject[defineAs];

        let proxy = new Proxy(original, {
          apply(target, thisArg, args) {
            return func.apply(thisArg, args);
          }
        });
        if (!propDes) {
          targetObject[propName] = proxy;
        } else {
          if (getOrSet) {
            propDes[getOrSet] = proxy;
          } else {
            if ("value" in propDes) {
              propDes.value = proxy;
            } else {
              return exportFunction(() => proxy, targetObject, `get $propName`);
            }
          }
          Object.defineProperty(targetObject, propName, propDes);
        }
        return proxy;
      } catch (e) {
        console.error(e, `setting $targetObject.$defineAs`, func);
      }
      return null;
    };
    let cloneInto = (obj, targetObject) => {
      return obj; // dummy for assignment
    };
    let script = document.createElement("script");
    script.text = `
    (() => {
      let patchWindow = $patchWindow;
      let cloneInto = $cloneInto;
      let exportFunction = $exportFunction;
      let env = ${JSON.stringify(env)};
      let eventId = ${JSON.stringify(eventId)};
      env.port = new ($Port)("page", "extension");
      ({
        patchWindow,
        exportFunction,
        cloneInto,
        eventId,
      }).patchWindow($patchingCallback, env);
    })();
    `;
    document.documentElement.insertBefore(script, document.documentElement.firstChild);
    script.remove();
    return port;
  }
  env.port = new Port("page", "extension");
  // win: window object to modify.
  // modifyTarget: callback to function that modifies the desired properties
  //                or methods. Callback must take target window as argument.
  function modifyWindow(win, modifyTarget) {
    try {
      win = win.wrappedJSObject || win;
      modifyTarget(win, env);
      modifyWindowOpenMethod(win, modifyTarget);
      modifyFramingElements(win, modifyTarget);
      // we don't need to modify win.opener, read skriptimaahinen notes
      // at https://forums.informaction.com/viewtopic.php?p=103754#p103754
    } catch (e) {
      if (e instanceof DOMException && e.name === "SecurityError") {
        // In case someone tries to access SOP restricted window.
        // We can just ignore this.
      } else throw e;
    }
  }

  function modifyWindowOpenMethod(win, modifyTarget) {
    let windowOpen = win.open;
    exportFunction(function(...args) {
      let newWin = windowOpen.call(this, ...args);
      if (newWin) modifyWindow(newWin, modifyTarget);
      return newWin;
    }, win, {defineAs: "open"});
  }

  function modifyFramingElements(win, modifyTarget) {
    for (let property of ["contentWindow", "contentDocument"]) {
      for (let iface of ["Frame", "IFrame", "Object"]) {
        let proto = win[`HTML$ifaceElement`].prototype;
        modifyContentProperties(proto, property, modifyTarget)
      }
    }
  }

  function modifyContentProperties(proto, property, modifyTarget) {
    let descriptor = Object.getOwnPropertyDescriptor(proto, property);
    let origGetter = descriptor.get;
    let replacementFn;

    if (property === "contentWindow") { replacementFn = function() {
      let win = origGetter.call(this);
      if (win) modifyWindow(win, modifyTarget);
      return win;
    }}
    if (property === "contentDocument") { replacementFn = function() {
      let document = origGetter.call(this);
      if (document && document.defaultView) modifyWindow(document.defaultView, modifyTarget);
      return document;
    }}

    descriptor.get = exportFunction(replacementFn, proto, {defineAs: `get $property`});
    Object.defineProperty(proto, property, descriptor);
  }

  modifyWindow(window, patchingCallback);
  return port;
}
diff -u

Code: Select all

@@ -126,7 +126,8 @@
   //                or methods. Callback must take target window as argument.
   function modifyWindow(win, modifyTarget) {
     try {
-      modifyTarget(win.wrappedJSObject || win, env);
+      win = win.wrappedJSObject || win;
+      modifyTarget(win, env);
       modifyWindowOpenMethod(win, modifyTarget);
       modifyFramingElements(win, modifyTarget);
       // we don't need to modify win.opener, read skriptimaahinen notes
@@ -140,7 +141,7 @@
   }
 
   function modifyWindowOpenMethod(win, modifyTarget) {
-    let windowOpen = win.wrappedJSObject ? win.wrappedJSObject.open : win.open;
+    let windowOpen = win.open;
     exportFunction(function(...args) {
       let newWin = windowOpen.call(this, ...args);
       if (newWin) modifyWindow(newWin, modifyTarget);
@@ -174,8 +175,7 @@
     }}
 
     descriptor.get = exportFunction(replacementFn, proto, {defineAs: `get $property`});
-    let wrappedProto = proto.wrappedJSObject || proto;
-    Object.defineProperty(wrappedProto, property, descriptor);
+    Object.defineProperty(proto, property, descriptor);
   }
 
   modifyWindow(window, patchingCallback);

Note about modifyWindowOpenMethod:

If the original window.open (windowOpen) is from contenscript context (win.open instead of win.wrappedJSObject.open) then with two consecutive calls on window open that return the same window, the latter will be SOP restricted.

Code: Select all

iframe1 = document.createElement('iframe');
iframe1.name = "iframe1";
document.body.appendChild(iframe1);
win1 = window.open("about:blank", "iframe1"); // window accessible
win1 = window.open("about:blank", "iframe1"); // window SOP restricted
No idea why. Does not happen with unmodified open or if original (windowOpen) is from page context. Bug on FireFox or me missing something?
Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Firefox/78.0

User avatar
Giorgio Maone
Site Admin
Posts: 9089
Joined: Wed Mar 18, 2009 11:22 pm
Location: Palermo - Italy
Contact:

Re: multiple patchWindow calls

Post by Giorgio Maone » Wed Apr 28, 2021 5:55 pm

Thanks, didn't manage to put it in rc3, but will go in rc4 for sure.
Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:89.0) Gecko/20100101 Firefox/89.0

User avatar
Giorgio Maone
Site Admin
Posts: 9089
Joined: Wed Mar 18, 2009 11:22 pm
Location: Palermo - Italy
Contact:

Re: multiple patchWindow calls

Post by Giorgio Maone » Wed Apr 28, 2021 6:41 pm

skriptimaahinen wrote:
Wed Apr 28, 2021 10:58 am

Code: Select all

@@ -174,8 +175,7 @@
     }}
 
     descriptor.get = exportFunction(replacementFn, proto, {defineAs: `get $property`});
-    let wrappedProto = proto.wrappedJSObject || proto;
-    Object.defineProperty(wrappedProto, property, descriptor);
+    Object.defineProperty(proto, property, descriptor);
   }
 
Just to be sure: is this change necessary? Why? Thanks!
Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:89.0) Gecko/20100101 Firefox/89.0

skriptimaahinen
Senior Member
Posts: 232
Joined: Wed Jan 10, 2018 7:37 am

Re: multiple patchWindow calls

Post by skriptimaahinen » Thu Apr 29, 2021 4:47 am

Since the window from which this prototype is taken is already wrapped, it is not necessary to wrap the prototype again, nor would it even work. The removed line would simply always reduce to wrappedProto = proto.

As an example: window.HTMLIFrameElement.prototype.wrappedJSObject === window.wrappedJSObject.HTMLIFrameElement.prototype, so it does not matter where the wrapping is done.

So, simply removing unnecessary line. This code really doesn't need to be any more complex than it already is. :)
Mozilla/5.0 (Windows NT 10.0; rv:78.0) Gecko/20100101 Firefox/78.0

User avatar
Giorgio Maone
Site Admin
Posts: 9089
Joined: Wed Mar 18, 2009 11:22 pm
Location: Palermo - Italy
Contact:

Re: multiple patchWindow calls

Post by Giorgio Maone » Thu Apr 29, 2021 7:56 am

skriptimaahinen wrote:
Thu Apr 29, 2021 4:47 am
Since the window from which this prototype is taken is already wrapped, it is not necessary to wrap the prototype again, nor would it even work. The removed line would simply always reduce to wrappedProto = proto.
Yes, you're right, but I had to double check myself because I was not sure that accessing a DOM property of an unwrapped object would create a new xray wrapper around it (which would make sense from a security standpoint, even quite inconvenient if you actually operate in the "page world").
Thank you again.
Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:89.0) Gecko/20100101 Firefox/89.0

Post Reply