Make ERRORS_HAVE_INVISIBLE_PROPERTIES repairable.

In <https://bugzilla.mozilla.org/show_bug.cgi?id=724768#c12> we now have
a specific list of the invisible properties on Error and a statement
that no other objects have this problem, so we can safely repair the
known problem. This makes Firefox 21 OK for SES as no-known-exploit.

@r5432

[kpreid2, 2013-05-29 18:51:02]

In <https://bugzilla.mozilla.org/show_bug.cgi?id=724768#c12> we now have
a specific list of the invisible properties on Error and a statement
that no other objects have this problem, so we can safely repair the
known problem. This makes Firefox 21 OK for SES as no-known-exploit.

[kpreid2, 2013-05-29 18:52:00]

I haven't thought too hard about whether this is perfect, because I'd first like
to get feedback on whether the basic approach is suitable (e.g. do we want to
repair gOPN at all? is the instanceof check safe and sufficient?).

It's unclear to me how much use there remains for errorInstanceBlacklist.

[MarkM, 2013-05-29 20:11:19]

Will review for real, probably this evening. Just wanted to say quickly here:
I'm happy to see this. I think it is an excellent approach.

[MarkM, 2013-05-30 16:44:11]

LGTM

https://codereview.appspot.com/9864044/diff/3001/src/com/google/caja/ses/repa...
File src/com/google/caja/ses/repairES5.js (right):

https://codereview.appspot.com/9864044/diff/3001/src/com/google/caja/ses/repa...
src/com/google/caja/ses/repairES5.js:3059: // the forEach binds this to the
error instance
Technically this terminology is wrong. forEach doesn't "bind" it in the sense of
making a new bound function. It does invoke it with the error instance bound to
"this", which accurately describes the point you're making.

https://codereview.appspot.com/9864044/diff/3001/src/com/google/caja/ses/repa...
src/com/google/caja/ses/repairES5.js:3066: // Note: not adequate for cross-frame
use, but we currently don't care
This will no longer be safe once SES allows mutation of [[Prototype]] on
extensible objects. Let's abstract this into an isError predicate, and for now
define isError as checking that objToString.call(object) ends with "Error]".
This will also not be an adequate check in SES-on-ES6, so we should still
include a note to revisit this then.

[kpreid2, 2013-05-31 20:52:47]

Please rereview as I didn't do exactly what you said.

https://codereview.appspot.com/9864044/diff/3001/src/com/google/caja/ses/repa...
File src/com/google/caja/ses/repairES5.js (right):

https://codereview.appspot.com/9864044/diff/3001/src/com/google/caja/ses/repa...
src/com/google/caja/ses/repairES5.js:3059: // the forEach binds this to the
error instance
On 2013/05/30 16:44:12, MarkM wrote:
> Technically this terminology is wrong. forEach doesn't "bind" it in the sense
> of making a new bound function. It does invoke it with the error instance
> bound to "this", which accurately describes the point you're making.

Rephrased.

https://codereview.appspot.com/9864044/diff/3001/src/com/google/caja/ses/repa...
src/com/google/caja/ses/repairES5.js:3066: // Note: not adequate for cross-frame
use, but we currently don't care
On 2013/05/30 16:44:12, MarkM wrote:
> This will no longer be safe once SES allows mutation of [[Prototype]] on
> extensible objects.

Good point.

(On the other hand, this problem is a SAFE_SPEC_VIOLATION, and the worst that
can be done to guest code using it could equally well be done if we permitted
proxies. We only care(d) about it because we thought it might interfere with
whitelisting.)

> Let's abstract this into an isError predicate, and for now
> define isError as checking that objToString.call(object) ends with "Error]".

Object.prototype.toString is clearly superior. For some reason, I was under the
impression it didn't work for Error, and I now have verified that that is false.

Done, except that I don't see great value in extracting it to a predicate.

> This will also not be an adequate check in SES-on-ES6

How so? ES6 is not allowing setting [[Class]], is it? Or will there ways to have
user-defined objects for which Object.prototype.toString varies?

[kpreid2, 2013-05-31 20:53:23]

In <https://bugzilla.mozilla.org/show_bug.cgi?id=724768#c12> we now have
a specific list of the invisible properties on Error and a statement
that no other objects have this problem, so we can safely repair the
known problem. This makes Firefox 21 OK for SES as no-known-exploit.

[MarkM, 2013-05-31 21:44:51]

LGTM

https://codereview.appspot.com/9864044/diff/3001/src/com/google/caja/ses/repa...
File src/com/google/caja/ses/repairES5.js (right):

https://codereview.appspot.com/9864044/diff/3001/src/com/google/caja/ses/repa...
src/com/google/caja/ses/repairES5.js:3066: // Note: not adequate for cross-frame
use, but we currently don't care
On 2013/05/31 20:52:47, kpreid2 wrote:
> How so? ES6 is not allowing setting [[Class]], is it? Or will there ways to
have
> user-defined objects for which Object.prototype.toString varies?

On ES6, Object.prototype.toString, instead of accessing the internal [[Class]]
property (which is going away anyway), will instead access a unique-symbol-named
property. Thus, any object can have any Object.prototype.toString.call(obj)
result. We will then need to revisit *all* the places where we're using
Object.prototype.toString as a brand check.

So yes, it is not worth abstracting this further until we abstract all such
brand checks.