Closed Bug 1363208 Opened 3 years ago Closed 1 year ago

Add security checking to WindowProxy and Location objects

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox66 --- fixed

People

(Reporter: jorendorff, Assigned: bzbarsky)

References

Details

Attachments

(10 files, 1 obsolete file)

47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
70.76 KB, text/plain
Details
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
Depends on: 1363211
Priority: -- → P2
Blocks: 1471496
When doing this, we'll need to use the incumbent global for the security checks until bug 1471496 gets fixed.
Blocks: 1471495
No longer blocks: same-compartment-realms
Blocks: 1514049
Blocks: 1514050
Depends on: 1515999
Blocks: 1160757
Depends on: 1516473
Depends on: 1353867
Depends on: 1516560
Depends on: 1516567
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.
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.
We can just check for a non-global object (so excluding Window) with cross-origin properties.
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...
Attachment #9033456 - Flags: review?(peterv)
Attachment #9033447 - Attachment description: Bug 1363208 part 2. Add a helper class for implementing the HTML requirements for cross-origin-accessible objects. r=bholley,jandem → Bug 1363208 part 2. Add a helper class for implementing the HTML requirements for cross-origin-accessible objects. r=bholley,jandem,peterv
Attachment #9033447 - Attachment description: Bug 1363208 part 2. Add a helper class for implementing the HTML requirements for cross-origin-accessible objects. r=bholley,jandem,peterv → Bug 1363208 part 2. Add a helper class for implementing the HTML requirements for cross-origin-accessible objects. r=jandem,peterv
Attachment #9033449 - Attachment description: Bug 1363208 part 3. Change nsOuterWindowProxy to inherit from MaybeCrossOriginObject. r=bholley → Bug 1363208 part 3. Change nsOuterWindowProxy to inherit from MaybeCrossOriginObject. r=peterv
Attachment #9033449 - Attachment description: Bug 1363208 part 3. Change nsOuterWindowProxy to inherit from MaybeCrossOriginObject. r=peterv → Bug 1363208 part 3. Change nsOuterWindowProxy to inherit from MaybeCrossOriginObject. r=peterv,jandem
Attachment #9033451 - Attachment description: Bug 1363208 part 5. Remove now-unnecessary named subframe handling from XrayWrapper. r=bholley → Bug 1363208 part 5. Remove now-unnecessary named subframe handling from XrayWrapper. r=peterv
Attachment #9033455 - Attachment description: Bug 1363208 part 9. Remove now-unused cross-origin Xray infrastructure. r=bholley → Bug 1363208 part 9. Remove now-unused cross-origin Xray infrastructure. r=peterv
Attachment #9033454 - Attachment description: Bug 1363208 part 8. Stop using cross-origin Xrays for Location. r=bholley → Bug 1363208 part 8. Stop using cross-origin Xrays for Location. r=peterv
Attachment #9033456 - Attachment is obsolete: true
Attachment #9033456 - Flags: review?(peterv)
Attachment #9034479 - Flags: review?(peterv)
Attachment #9033450 - Attachment description: Bug 1363208 part 4. Stop using cross-origin Xrays for WindowProxy. r=bholley → Bug 1363208 part 4. Stop using cross-origin Xrays for WindowProxy. r=peterv,jandem
Depends on: 1518812
Pushed by bzbarsky@mozilla.com:
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
Attachment #9034479 - Flags: review?(peterv)
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/14975 for changes under testing/web-platform/tests
Assignee: nobody → bzbarsky
You need to log in before you can comment on or make changes to this bug.