Array[@@Species] Security Discussion

[natashenka]

There have been a lot of security issues in Array[@@species] (see: https://docs.google.com/presentation/d/11fkQeEisoszNGF8SrautVT1ltSnsQBWRxJ4usoc-g_o/edit#slide=id.g2b34aaab4a_1_0) and usage is low (see: https://www.chromestatus.com/metrics/feature/timeline/popularity/1391). What is the appetite for deprecating this feature? Or are there other ways to reduce the security impact without changing functionality?

[domenic]

What do you mean by "deprecating"?

Would you retain the lookup of this.constructor, or is this discussion also about removing that?

[natashenka]

I don't have a clear idea on this yet, there's a few options. I think the least invasive one would be to have Array[@@species] always return the Array constructor for an Array and the Object constructor for an Object. Most of the security risk and implementation complexity of @@species comes from the call to untrusted script in creating the Array, and the unpredictability of what is returned. So it would need to be a solution that means the engine constructs the object returned from an Array method call without user script involvement.

[littledan]

I believe a change here would only make sense if we changed the semantics to the following: all Array methods like map create an Array instance rather than a subclass instance, i.e., removing the use of this.constructor. If the security issues were about weird things happening when a non-Array is produced, then we'd want to address that avenue as well.

I don't have a great idea how we could deprecate this feature; one way would be, for a period of time, implementations could place warning messages whenever they see non-Arrays being produced, and then implementations would switch to the ES5.1 semantics. Chrome shipped ES2015 without such a mechanism for the most part, so I'm not sure if it would be necessary here.

Array subclassing might be the most useful part of ES2015 "subclassable builtins"--at least it's the one that makes the most intuitive sense to me. If we disable the @@species here, maybe we want to rethink all the cases where ES2015 added the pattern of creating instances of this.constructor[@@species], not just this one.

[domenic]

I personally find @@species useless but this.constructor a good change, so removing that would be, IMO, throwing the baby out with the bathwater.

[littledan]

I don't think any security (or performance) issue would be resolved if we kept this.constructor.

[bmeck]

Can we comment on this.constructor solution a bit. I've started looking towards doing:

class Foo {
  constructor() {...}
}
Object.setPrototypeOf(Foo.prototype, null);
delete Foo.prototype.constructor;

Which is fine if not doing a real property access to find this.constructor, I just want to be sure. Also, this could get weird if I delete the constructor but the superclass has one (not true for my uses so far)?

[littledan]

@bmeck I imagine that if we go for the this.constructor solution, we'll keep the thing where there's a "default" class to fall back to the ES5.1 semantics if @@species doesn't work. For your use case, why not set Foo.prototype.constructor = undefined if you're worried about superclasses (and apparently don't care about other accesses to the constructor)?

[bmeck]

@littledan I still don't know the full spec of the this.constructor solution, is there a write up. Is it just property access done when running things like @@species using functions Array.prototype.map? What does the following do?

const obj = Object.setPrototypeOf([1], {
  constructor: Error
});
Array.prototype.map.call(obj, _ => _);

Do I get an Error object back? An explanation of the full behavior or link to behavior explanation would be helpful.

[littledan]

To be concrete, I'm imagining deleting step 7 of ArraySpeciesCreate. In that case, you would get an Error object back! Note that you can observe this strange category of behavior today if you just replaced Error with Promise or something else that had @@species.