0

The objective

I've been working on a polyfill of sorts for the Trusted Types API. Part of what Trusted Types does is update handling of "injection sinks" such as Element.prototype.innerHTML and new Function() to accept TrustedHTML and TrustedScript instead of strings. The polyfill to create the new classes is already done, and I'm adding the additional script to replace injection sink methods.

Note: I'm doing this following a spec and recreating the behavior in Chromium browsers. I think that the added security justifies changing the built-in methods if not already supported. It's separate from the polyfill itself in a separate script (harden.js).

An Example

const sanitizer = new Sanitizer();

const policy = trustedTypes.createPolicy('default', {
  // This is used for `el.innerHTML`, `iframe.srcdoc`, and a few others
  createHTML: input => sanitizer.sanitizeFor('div', input).innerHTML,

  // This is used for `eval()`, `new Function()`, `script.text`, etc
  createScript: () => trustedTypes.emptyScript,

  // This is mainly for setting `script.src`/`script.setAttribute('src', src)`
  createScriptURL: input => /* Something to check that the URL is allowed */
});

document.getElementById('foo').innerHTML = '<button onclick="alert(document.cookie)">Btn</button>';
document.getElementById('foo').innerHTML; // '<button>Btn</button>'
eval('alert(location.href)'); // Does nothing... Same as `eval('')`
const func = new Function('alert(location.href)'); // This is what I am wanting to change
func(); // This should do nothing, similar to `eval()`
fetch instanceof Function; // This should remain true

This all works without issue, but I can't seem to update new Function() in a way that supports/requires TrustedScript while still preserving fetch instanceof Function.

Failed Ideas

Both of these use a createScript() method that's not particularly important here. It checks for support, checks the type of input (string or TrustedScript), and tries trustedTypes.defaultPolicy.createScript(input) as needed.

// This doesn't work and is never even called

const func = globalThis.Function.prototype.constructor;

globalThis.Function.prototype.constructor = function Function(...args) {
    if (args.length === 0) {
        return func.call(this);
    } else {
        const funcBody = createScript(args.pop());
        return func.apply(this, [...args, funcBody.toString()]);
    }
};

... or

// This fails `fetch instanceof Function`

const NativeFunction = globalThis.Function;

globalThis.Function = class Function extends NativeFunction {
  constructor(...args) {
    if (args.length === 0) {
      super();
    } else {
      const funcBody = createScript(args.pop());
      super(...args, funcBody.toString());
    }
  }
};

To Summarize

I'm trying to replace/update new Function() to support/require TrustedScript in a way that does not break fetch instanceof Function. Everything else was doable since it required just updating certain methods / setters, but new Function() is a constructor and I don't know how to change it without changing Function itself.

Chris
  • 3,833
  • 3
  • 14
  • 11

1 Answers1

1

The instanceof operator checks whether the lhs has the .prototype of th rhs in its prototype chain. So when you replace Function, you'll need to ensure that Function.prototype stays the same. You can do that using

const NativeFunction = globalThis.Function;

globalThis.Function = function Function(...args) {
    if (args.length === 0) {
        return NativeFunction.call(this);
    } else {
        const funcBody = createScript(args.pop());
        return NativeFunction.call(this, ...args, funcBody.toString());
    }
};
globalThis.Function.prototype = NativeFunction.prototype;
NativeFunction.prototype.constructor = globalThis.Function;

Btw, using NativeFunction.call is a) slightly insecure, since it could be overwritten to gain access to the NativeFunction, and b) slightly incorrect since it'll break subclassing Function. You could either just call NativeFunction(...args, funcBody.toString()); (given the thisArg isn't used anyway) or better use Reflect.construct(NativeFunction, [...args, funcBody.toString()], new.target) (assuming you actually use a save non-overwritable reference to Reflect.construct).

And then there's other ways to obtain a reference to the NativeFunction that you'll need to prohibit it you want to make this really secure, ranging from Object.getPrototypeOf(AsyncFunction)/Object.getPrototypeOf(GeneratorFunction) to using iframes to access a fresh realm. This will evolve into SES if you really mean it…

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Edit: That works, and I've gone with `NativeFunction()` instead of `NativeFunction.call()`. I chose to not rely on `Reflect` though. At least pending a review of compatibility, which is especially important in a polyfill. At least in Firefox, `AsyncFunction` and `GeneratorFunction` do not exist as constructors, and the spec doesn't mention them at all, so I'll not be worrying about them for now. And as far as SES... I suppose you could see this as a spec-following trade-off aiming for simplicity and best load times. I'm kinda at a race against any malicious scripts injected... – Chris Mar 01 '23 at 20:54
  • As such, I'm also not too invested in what can be done via ` – Chris Mar 01 '23 at 21:02
  • `AsyncFunction` and `GeneratorFunction` do exist (see the spec!), they're just not exposed as globals, but can easily be accessed from `(async()=>{}).constructor` and `(function*(){}).constructor`. If you're trying to safeguard `Function`, you'll also need to safeguard them! (And `AsyncGeneratorFunction` and possibly more…) – Bergi Mar 01 '23 at 22:19