1363208 - Add security checking to WindowProxy and Location objects

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement, P2)

Description

[Jason Orendorff [:jorendorff]]

With same-compartment realms, we need a new way to enforce security
checks during same-compartment cross-realm access.

This is a matter of following the spec:

>     <bz> jorendorff: well, we have a spec.
>     <bz> jorendorff: lemme pull it up
>     <bz> jorendorff: https://html.spec.whatwg.org/multipage/browsers.html#location-getprototypeof
>          and following
>     <bz> jorendorff: some of those internal methods just are, but some do sPlatformObjectSameOrigin(
>     <bz> jorendorff: We'd just do that
>     <bz> jorendorff: So in [[GetPrototypeOf]], [[GetOwnProperty]], [[DefineOwnProperty]],
>          [[Get]], [[Set]], [[Delete]], [[OwnPropertyKeys]]
>     <bz> jorendorff: (in terms of our code, in the corresponding proxy handler bits
>     <bz> jorendorff: The windowproxy bits are at
>          https://html.spec.whatwg.org/multipage/browsers.html#windowproxy-getprototypeof
>          and following
>     <bz> jorendorff: but it's the same list

Comment 1

[Boris Zbarsky [:bzbarsky]]

When doing this, we'll need to use the incumbent global for the security checks until bug 1471496 gets fixed.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1363208 part 1. Add a MaybeWrapObject function that works on JSObject* instead of JS::Value. r=peterv

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1363208 part 2. Add a helper class for implementing the HTML requirements for cross-origin-accessible objects. r=jandem,peterv

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1363208 part 3. Change nsOuterWindowProxy to inherit from MaybeCrossOriginObject. r=peterv,jandem

The cross-origin named window code in
nsOuterWindowProxy::getOwnPropertyDescriptor is mostly copied from
XrayWrapper::getPropertyDescriptor, with some minor changes because we can't
assume some work that CrossOriginXrayWrapper does.  The getPropertyDescriptor
version will go away in a later patch in this stack.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1363208 part 4. Stop using cross-origin Xrays for WindowProxy. r=peterv,jandem

The change to test_bug440572.html is due to a behavior change.  Specifically,
before this change, any IDL-declared property, even one not exposed
cross-origin, would prevent named frames with that name being visible
cross-origin.  The new behavior is that cross-origin-exposed IDL properties
prevent corresponding frame names from being visible, but ones not exposed
cross-origin don't.  This matches the spec and other browsers.

Same thing for the changes to test_bug860494.xul.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1363208 part 5. Remove now-unnecessary named subframe handling from XrayWrapper. r=peterv

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1363208 part 6. Remove the NonOrdinaryGetPrototypeOf annotation. r=peterv

We can just check for a non-global object (so excluding Window) with cross-origin properties.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1363208 part 7. Change the Location binding to inherit from MaybeCrossOriginObject. r=peterv

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1363208 part 8. Stop using cross-origin Xrays for Location. r=peterv

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1363208 part 9. Remove now-unused cross-origin Xray infrastructure. r=peterv

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: Generated LocationBinding.cpp after the changes

Peter, I don't see a way to attach files to Phabricator revisions, and pasting this whole thing in the revision would probably not be a good idea...

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: Generated LocationBinding.cpp after cross-compartment fixes

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8c6fcfca6421
part 1.  Add a MaybeWrapObject function that works on JSObject* instead of JS::Value.  r=peterv,bholley
https://hg.mozilla.org/integration/autoland/rev/52f7c0595d0d
part 2.  Add a helper class for implementing the HTML requirements for cross-origin-accessible objects.  r=jandem,peterv
https://hg.mozilla.org/integration/autoland/rev/9b1badc02fd1
part 3.  Change nsOuterWindowProxy to inherit from MaybeCrossOriginObject.  r=peterv,jandem
https://hg.mozilla.org/integration/autoland/rev/2e31b4d57c6a
part 4.  Stop using cross-origin Xrays for WindowProxy.  r=peterv,jandem
https://hg.mozilla.org/integration/autoland/rev/7faa84f9f73a
part 5.  Remove now-unnecessary named subframe handling from XrayWrapper.  r=peterv
https://hg.mozilla.org/integration/autoland/rev/c825004b9059
part 6.  Remove the NonOrdinaryGetPrototypeOf annotation.  r=peterv
https://hg.mozilla.org/integration/autoland/rev/140c8b32490c
part 7.  Change the Location binding to inherit from MaybeCrossOriginObject.  r=peterv
https://hg.mozilla.org/integration/autoland/rev/d4d779afb736
part 8.  Stop using cross-origin Xrays for Location.  r=peterv
https://hg.mozilla.org/integration/autoland/rev/dbab9ee37db1
part 9.  Remove now-unused cross-origin Xray infrastructure.  r=peterv

Comment 14

[Web Platform Test Sync Bot (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/14975 for changes under testing/web-platform/tests

Comment 15

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/8c6fcfca6421
https://hg.mozilla.org/mozilla-central/rev/52f7c0595d0d
https://hg.mozilla.org/mozilla-central/rev/9b1badc02fd1
https://hg.mozilla.org/mozilla-central/rev/2e31b4d57c6a
https://hg.mozilla.org/mozilla-central/rev/7faa84f9f73a
https://hg.mozilla.org/mozilla-central/rev/c825004b9059
https://hg.mozilla.org/mozilla-central/rev/140c8b32490c
https://hg.mozilla.org/mozilla-central/rev/d4d779afb736
https://hg.mozilla.org/mozilla-central/rev/dbab9ee37db1