On Wed, Apr 29, 2015 at 1:37 PM, Mark S. Miller <erights at google.com> wrote:

> On Wed, Apr 29, 2015 at 10:26 AM, C. Scott Ananian <ecmascript at cscott.net>
> wrote:
>
>> On Wed, Apr 29, 2015 at 1:00 PM, Mark S. Miller <erights at google.com>
>> wrote:
>>
>>> The invariant I am interested in:
>>>
>>> In a realm where we (the trusted defender who runs first) make Promise
>>> defensive as follows
>>>
>>> * Freeze everything primordial, as SES does
>>>
>>> * Make a DefensivePromise subclass of Promise that differs minimally,
>>> hopefully only by ensuring that its instances are frozen.
>>>
>>> * "Promise = DefensivePromise;" do "Promise" below refers to
>>> DefensivePromise
>>>
>>> * Freezing whitelisted global properties, as SES currently does for ES5
>>> globals, but for ES6 including "Promise"
>>>
>>>
>>> then it must be the case that
>>>
>>>     Promise.resolve(anything).then(anycallback)
>>>
>>> for an anything provided by a potential attacker, when executed in the
>>> middle of a turn does not call callback during that turn. If it calls
>>> anycallback at all, it calls it back *as* a later turn, i.e., in a later
>>> turn starting from an empty stack.
>>>
>>
>> How about:
>> ```
>> var goodPromises = new WeakSet();
>> class DefensivePromise {
>>   constructor(x) {
>>     super(x);
>>     Object.freeze(this);
>>     // check this.constructor here if you are paranoid.
>>     goodPromises.add(this);
>>   }
>>   resolve(x) {
>>     if (goodPromises.has(x)) {
>>       return super.resolve(x);
>>     }
>>     return new DefensivePromise(function(r){r(x);});
>>   }
>> }
>> ```
>> Doesn't seem like this needs special support in the Promise spec.
>>
>> Note that the `goodPromises` set won't be fooled by passing in
>> `DefensivePromise` as `new.target` to `Promise` without actually running
>> the `DefensivePromise` constructor.
>>   --scott
>>
>
> Isn't this still vulnerable to the Promise.resolve attack? IIUC, this
> attack enables the attacker to cause this.constructor to lie, so how would
> checking it help?
>

Because I only allow `DefensivePromise.resolve` to check `this.constructor`
(via the call to super.resolve) if I know that `this.constructor` is not
lying (because I froze `this.constructor` before adding it to
`goodPromises`).  All other promises get wrapped in a new
`DefensivePromise`, which should enforce the desired "not on this turn"
invariant.

In this particular example, I'm not exporting DefensivePromise, just using
it internally to sanitize "anything provided by a potential attacker"
(while preserving efficiency if the attacker is in good faith giving my
promises back to me).  If you want to export `DefensivePromise` and allow
malicious code to subclass it, then you should perform some additional
checks after freezing and before adding to `goodPromises`, perhaps
something like:
```
  constructor(x) {
    super(x);
    Object.freeze(this);
    if (this.constructor==DefensivePromise && this.then ===
DefensivePromise.prototype.then && /*) {
      goodPromises.add(this);
    }
  }
```
You probably need to make the test a little bit more careful, in case
`then` is a malicious getter or something like that.  But my point is that
you can do all this in userland, you don't need to expose its complexity or
bake any of it into the Promise spec.
 --scott
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20150429/cdeb3b3e/attachment.html>

On Wed, Apr 29, 2015 at 1:37 PM, Mark S. Miller <erights at google.com> wrote:

> Isn't this still vulnerable to the Promise.resolve attack? IIUC, this
> attack enables the attacker to cause this.constructor to lie, so how would
> checking it help?
>

Because I only allow `DefensivePromise.resolve` to check `this.constructor`
(via the call to super.resolve) if I know that `this.constructor` is not
lying (because I froze `this.constructor` before adding it to
`goodPromises`).  All other promises get wrapped in a new
`DefensivePromise`, which should enforce the desired "not on this turn"
invariant.

In this particular example, I'm not exporting DefensivePromise, just using
it internally to sanitize "anything provided by a potential attacker"
(while preserving efficiency if the attacker is in good faith giving my
promises back to me).  If you want to export `DefensivePromise` and allow
malicious code to subclass it, then you should perform some additional
checks after freezing and before adding to `goodPromises`, perhaps
something like:
```
  constructor(x) {
    super(x);
    Object.freeze(this);
    if (this.constructor==DefensivePromise && this.then ===
DefensivePromise.prototype.then && /*) {
      goodPromises.add(this);
    }
  }
```
You probably need to make the test a little bit more careful, in case
`then` is a malicious getter or something like that.  But my point is that
you can do all this in userland, you don't need to expose its complexity or
bake any of it into the Promise spec.