Whitelists non-std %IteratorPrototype%.next (issue 222570043 by erights@gmail.com)

[eri...@gmail.com]

Reviewers: kpreid2,

Description:
Fixes https://code.google.com/p/google-caja/issues/detail?id=1962 by
working around https://bugzilla.mozilla.org/show_bug.cgi?id=1152550,
which is currently causing cross-frame for/in to fail on some FF
betas.

Also change the whitelisting of %Generator%.prototype(.next, .return,
.throw) from t to *, since inheriting these should be safe.

NOTE: Not tested yet. Not to be submitted until tested.

Please review this at https://codereview.appspot.com/222570043/

Affected files (+8, -4 lines):
M src/com/google/caja/ses/whitelist.js


Index: src/com/google/caja/ses/whitelist.js
===================================================================
--- src/com/google/caja/ses/whitelist.js (revision 5722)
+++ src/com/google/caja/ses/whitelist.js (working copy)
@@ -121,7 +121,10 @@
anonIntrinsics: {
ThrowTypeError: {},
IteratorPrototype: {
- constructor: false // suppress inherited '*'
+ constructor: false, // suppress inherited '*'
+ // See https://bugzilla.mozilla.org/show_bug.cgi?id=1152550
+ // and
https://code.google.com/p/google-caja/issues/detail?id=1962
+ next: '*'
},
ArrayIteratorPrototype: {},
StringIteratorPrototype: {},
@@ -130,9 +133,10 @@
GeneratorFunction: {
prototype: {
prototype: {
- next: t,
- 'return': t,
- 'throw': t
+ next: *, // redundant, but IteratorPrototype.next isn't std
+ // and so might disappear in the future.
+ 'return': *,
+ 'throw': *
}
}
},

[eri...@gmail.com]

Fixes https://code.google.com/p/google-caja/issues/detail?id=1962 by
working around https://bugzilla.mozilla.org/show_bug.cgi?id=1152550,
which is currently causing cross-frame for/in to fail on some FF
betas.

Also change the whitelisting of %Generator%.prototype(.next, .return,
.throw) from t to *, since inheriting these should be safe.

NOTE: Not tested yet. Not to be submitted until tested.

https://codereview.appspot.com/222570043/

[kpr...@google.com]

LGTM


https://codereview.appspot.com/222570043/diff/40001/src/com/google/caja/ses/whitelist.js
File src/com/google/caja/ses/whitelist.js (right):

https://codereview.appspot.com/222570043/diff/40001/src/com/google/caja/ses/whitelist.js#newcode135
src/com/google/caja/ses/whitelist.js:135: prototype: {
Maybe a brief comment here about what
GeneratorFunction.prototype.prototype is, for clarity.

https://codereview.appspot.com/222570043/diff/40001/src/com/google/caja/ses/whitelist.js#newcode136
src/com/google/caja/ses/whitelist.js:136: next: *, // redundant, but
IteratorPrototype.next isn't std
quoted '*' (3 times)

https://codereview.appspot.com/222570043/

[eri...@gmail.com]

https://codereview.appspot.com/222570043/diff/40001/src/com/google/caja/ses/whitelist.js
File src/com/google/caja/ses/whitelist.js (right):

https://codereview.appspot.com/222570043/diff/40001/src/com/google/caja/ses/whitelist.js#newcode135
src/com/google/caja/ses/whitelist.js:135: prototype: {

On 2015/04/13 18:35:10, kpreid_google wrote:
> Maybe a brief comment here about what
GeneratorFunction.prototype.prototype is,
> for clarity.

Done.

https://codereview.appspot.com/222570043/diff/40001/src/com/google/caja/ses/whitelist.js#newcode136
src/com/google/caja/ses/whitelist.js:136: next: *, // redundant, but
IteratorPrototype.next isn't std

On 2015/04/13 18:35:10, kpreid_google wrote:
> quoted '*' (3 times)

Done.

https://codereview.appspot.com/222570043/

[kpr...@google.com]

still LGTM


https://codereview.appspot.com/222570043/diff/60001/src/com/google/caja/ses/whitelist.js
File src/com/google/caja/ses/whitelist.js (right):

https://codereview.appspot.com/222570043/diff/60001/src/com/google/caja/ses/whitelist.js#newcode136
src/com/google/caja/ses/whitelist.js:136: // the %Generator% intrinsic,
which all generator function
This makes me wonder if cajaVM.anonIntrinsics.Generator should exist.
After all, that is itself an anonymous intrinsic, which just happens to
be reachable from another.

https://codereview.appspot.com/222570043/

[eri...@gmail.com]

Many changes since last time. Now with tests, so a submission candidate.

PTAL.

https://codereview.appspot.com/222570043/diff/60001/src/com/google/caja/ses/whitelist.js
File src/com/google/caja/ses/whitelist.js (right):

https://codereview.appspot.com/222570043/diff/60001/src/com/google/caja/ses/whitelist.js#newcode136
src/com/google/caja/ses/whitelist.js:136: // the %Generator% intrinsic,
which all generator function

On 2015/04/14 17:42:27, kpreid_google wrote:
> This makes me wonder if cajaVM.anonIntrinsics.Generator should exist.
After all,
> that is itself an anonymous intrinsic, which just happens to be
reachable from
> another.

If we did, I think we would fail the

fail('primordial reachable through multiple paths');

test, though I haven't tested this. Perhaps this argues against that
internal test, but on balance I'd rather keep it.

https://codereview.appspot.com/222570043/