Fix breach due to exposed taming constructors exposing arbitrary property access.

Several taming constructors (function TameFoo(feralFoo) {...}) in Domado
were not protected via inertCtor (which replaces .prototype.constructor
with a useless function), which resulted in guest code being able to
invoke them.

Some of the taming constructors called makeDOMAccessible on an argument,
leading to guest code being able to cause that; thus allowing ES5/3 guest
code to read or write nonwhitelisted properties on objects the guest
has access to.

Together, this allowed a complete breach, permitting arbitrary
manipulation of the host page.

* All taming constructors now use inertCtor properly.
* There is now a test which checks for this problem broadly.
* makeDOMAccessible now refuses to operate on objects which are already
  ES5/3 objects (have a v___ property). Added a test for this. Guest
  code still should not be allowed to obtain makeDOMAccessible, but this
  should make it less hazardous if that does occur.
* Fixed lack of comment in jsUnit error messages for assertRoughlyEquals
  and assertContains.

The Firefox quirk referred to in the previous comments of
makeDOMAccessible appears to no longer be an issue (FF 16.0.2).

[metaweta, 2012-11-21 00:49:17]

LGTM

https://codereview.appspot.com/6843096/diff/3001/tests/com/google/caja/plugin...
File tests/com/google/caja/plugin/es53-test-domado-dom-guest.html (right):

https://codereview.appspot.com/6843096/diff/3001/tests/com/google/caja/plugin...
tests/com/google/caja/plugin/es53-test-domado-dom-guest.html:2018:
constructorCallable: extras.constructorCallable || false
maybe !!extras.constructorCallable

https://codereview.appspot.com/6843096/diff/3001/tests/com/google/caja/plugin...
tests/com/google/caja/plugin/es53-test-domado-dom-guest.html:2074: /^function
(?:Object|Function|FakeFunction|Array)\(/
Have you tested this regex on all supported browsers?  I wouldn't be surprised
if some have whitespace between the function name and the arg list.

https://codereview.appspot.com/6843096/diff/3001/tests/com/google/caja/plugin...
tests/com/google/caja/plugin/es53-test-domado-dom-guest.html:2087: break ok;
Why not return?

[ihab.awad, 2012-11-21 01:02:04]

https://codereview.appspot.com/6843096/diff/3001/tests/com/google/caja/plugin...
File tests/com/google/caja/plugin/es53-test-domado-dom-guest.html (right):

https://codereview.appspot.com/6843096/diff/3001/tests/com/google/caja/plugin...
tests/com/google/caja/plugin/es53-test-domado-dom-guest.html:2008: [[], new
Option()],
Fwiw: The above is really hard to read and understand; a more JSON-esque
structure might be more understandable.

[kpreid2, 2012-11-21 01:03:14]

New snapshot.

https://codereview.appspot.com/6843096/diff/3001/tests/com/google/caja/plugin...
File tests/com/google/caja/plugin/es53-test-domado-dom-guest.html (right):

https://codereview.appspot.com/6843096/diff/3001/tests/com/google/caja/plugin...
tests/com/google/caja/plugin/es53-test-domado-dom-guest.html:2018:
constructorCallable: extras.constructorCallable || false
On 2012/11/21 00:49:17, metaweta wrote:
> maybe !!extras.constructorCallable

Thought about it, liked this better for "if not supplied" idiom.

https://codereview.appspot.com/6843096/diff/3001/tests/com/google/caja/plugin...
tests/com/google/caja/plugin/es53-test-domado-dom-guest.html:2074: /^function
(?:Object|Function|FakeFunction|Array)\(/
On 2012/11/21 00:49:17, metaweta wrote:
> Have you tested this regex on all supported browsers?  I wouldn't be surprised
> if some have whitespace between the function name and the arg list.

Changed \( to \b.

https://codereview.appspot.com/6843096/diff/3001/tests/com/google/caja/plugin...
tests/com/google/caja/plugin/es53-test-domado-dom-guest.html:2087: break ok;
On 2012/11/21 00:49:17, metaweta wrote:
> Why not return?

Replaced the whole thing with expectFailure.

[kpreid2, 2012-11-21 01:10:08]

https://codereview.appspot.com/6843096/diff/3001/tests/com/google/caja/plugin...
File tests/com/google/caja/plugin/es53-test-domado-dom-guest.html (right):

https://codereview.appspot.com/6843096/diff/3001/tests/com/google/caja/plugin...
tests/com/google/caja/plugin/es53-test-domado-dom-guest.html:2008: [[], new
Option()],
On 2012/11/21 01:02:04, ihab.awad wrote:
> Fwiw: The above is really hard to read and understand; a more JSON-esque
> structure might be more understandable.

I admit to having designed it for density. I thought that a full jsonian
structure would be overly verbose; I was perhaps influenced by that this was
initially replacing the hardcoded assertInstanceof code in the older version of
testInterfaceGlobals. Will think about it tomorrow if actually deploying the
security patch doesn't get in the way.

[kpreid2, 2012-12-03 20:39:46]

Several taming constructors (function TameFoo(feralFoo) {...}) in Domado
were not protected via inertCtor (which replaces .prototype.constructor
with a useless function), which resulted in guest code being able to
invoke them.

Some of the taming constructors called makeDOMAccessible on an argument,
leading to guest code being able to cause that; thus allowing ES5/3 guest
code to read or write nonwhitelisted properties on objects the guest
has access to.

Together, this allowed a complete breach, permitting arbitrary
manipulation of the host page.

* All taming constructors now use inertCtor properly.
* There is now a test which checks for this problem broadly.
* makeDOMAccessible now refuses to operate on objects which are already
  ES5/3 objects (have a v___ property). Added a test for this. Guest
  code still should not be allowed to obtain makeDOMAccessible, but this
  should make it less hazardous if that does occur.
* Fixed lack of comment in jsUnit error messages for assertRoughlyEquals
  and assertContains.

The Firefox quirk referred to in the previous comments of
makeDOMAccessible appears to no longer be an issue (FF 16.0.2).

[kpreid2, 2012-12-03 20:41:00]

https://codereview.appspot.com/6843096/diff/3001/tests/com/google/caja/plugin...
File tests/com/google/caja/plugin/es53-test-domado-dom-guest.html (right):

https://codereview.appspot.com/6843096/diff/3001/tests/com/google/caja/plugin...
tests/com/google/caja/plugin/es53-test-domado-dom-guest.html:2008: [[], new
Option()],
Replaced with explicit record structure in new snapshot.

[ihab.awad, 2012-12-03 20:57:34]

https://codereview.appspot.com/6843096/diff/11001/tests/com/google/caja/plugi...
File tests/com/google/caja/plugin/es53-test-domado-dom-guest.html (right):

https://codereview.appspot.com/6843096/diff/11001/tests/com/google/caja/plugi...
tests/com/google/caja/plugin/es53-test-domado-dom-guest.html:2081: //
TODO(kpreid): Add opaque and foreign nodes as test cases here.
Yippee! :) Thanks!