Protect Domado accessors that return feral props directly.

This is a cheap fix for the known issue with <form> elements that
should be replaced with a more robust, general solution.

Each Domado accessor that simply returned a value from the feral node
now coerces it to the expected type first.

https://github.com/google/caja/commit/d34290a06cabffa02d7cd509223134ae1d9b2576

[MarkM, 2015-04-30 17:02:27]

LGTM

https://codereview.appspot.com/235830043/diff/1/src/com/google/caja/plugin/do...
File src/com/google/caja/plugin/domado.js (right):

https://codereview.appspot.com/235830043/diff/1/src/com/google/caja/plugin/do...
src/com/google/caja/plugin/domado.js:5583: return String(privates.feral.align);
For stylistic consistency with the rest of Caja, should this be ''+..." rather
than "String(...)"?

[kpreid_google, 2015-04-30 17:13:05]

https://codereview.appspot.com/235830043/diff/1/src/com/google/caja/plugin/do...
File src/com/google/caja/plugin/domado.js (right):

https://codereview.appspot.com/235830043/diff/1/src/com/google/caja/plugin/do...
src/com/google/caja/plugin/domado.js:5583: return String(privates.feral.align);
On 2015/04/30 17:02:26, MarkM wrote:
> For stylistic consistency with the rest of Caja, should this be ''+..." rather
> than "String(...)"?

The value is/should-be a string, not a number. + coerces to number. No?

[MarkM, 2015-04-30 17:19:23]

On 2015/04/30 17:13:05, kpreid_google wrote:
>
https://codereview.appspot.com/235830043/diff/1/src/com/google/caja/plugin/do...
> File src/com/google/caja/plugin/domado.js (right):
> 
>
https://codereview.appspot.com/235830043/diff/1/src/com/google/caja/plugin/do...
> src/com/google/caja/plugin/domado.js:5583: return
String(privates.feral.align);
> On 2015/04/30 17:02:26, MarkM wrote:
> > For stylistic consistency with the rest of Caja, should this be ''+..."
rather
> > than "String(...)"?
> 
> The value is/should-be a string, not a number. + coerces to number. No?

I confused myself with my own typography. Trying again:

For stylistic consistency with the rest of Caja, should this be 

    ''+...

rather than

    String(...)

? Note that '' is two single quotes, not one double quote.

[kpreid_google, 2015-04-30 17:25:26]

On 2015/04/30 17:19:23, MarkM wrote:
> For stylistic consistency with the rest of Caja, should this be 
> 
>     ''+...
> 
> rather than
> 
>     String(...)
> 
> ? Note that '' is two single quotes, not one double quote.

I would agree with you, but Domado uses String() all over and particularly right
next to this code. Perhaps that should be changed, but not as part of this.

OK to continue?

[MarkM, 2015-04-30 17:30:46]

On 2015/04/30 17:25:26, kpreid_google wrote:
> On 2015/04/30 17:19:23, MarkM wrote:
> > For stylistic consistency with the rest of Caja, should this be 
> > 
> >     ''+...
> > 
> > rather than
> > 
> >     String(...)
> > 
> > ? Note that '' is two single quotes, not one double quote.
> 
> I would agree with you, but Domado uses String() all over and particularly
right
> next to this code. Perhaps that should be changed, but not as part of this.
> 
> OK to continue?

y. Still LGTM.

[kpreid_google, 2016-01-28 18:46:54]

Forwarding here for the record:

On Thu, Jan 28, 2016 at 10:25 AM, Mike Stay <metaweta@gmail.com> wrote:
> How does coercing length, align, frameBorder, and defaultMuted help
> with the form issue?

Coercing .length  on forms prevents <input name="length"> from leaking.
The other properties aren't known security issues because they are specific to
other node types, but these changes make it so we never pass _any_ value
uncoerced to the guest side.