Define security around Window, WindowProxy, and Location properly #638

annevk · 2016-02-06T13:00:57Z

This is a relative large change to the way Window, WindowProxy, and
Location objects work, at least from a standards’ perspective. This
should more closely match implementations and define all the relevant
details.

A rough summary of the changes:

Window indexed getters have moved to WindowProxy.
WindowProxy is now a JavaScript exotic object and it handles all the
security aspects for Window so Window can become an ordinary object.
Cross-origin named properties for Window are now exposed (though on
WindowProxy). This addresses Child browsing context names leak across origins #544 and
https://www.w3.org/Bugs/Public/show_bug.cgi?id=21674 which that PR was
intended to replace.
Location is now an exotic object with all its internal methods
overridden to handle the cross-origin security aspects.
Location no longer uses Unforgeable on the interface, allowing that
to be removed from IDL. (It moved to all of its properties instead and
the remaining details are defined through prose.)

This should also address these bugs:

annevk · 2016-02-06T13:04:00Z

Things I need to look into:

Now WindowProxy handles indexed properties directly, we should expose them through [[OwnPropertyKeys]] too. Currently we only do that for the cross-origin case.
I need to get Editorial: Introduce OrdinaryGet/Set/OwnPropertyKeys and use them tc39/ecma262#356 landed and add some references.

Things that made me ponder:

Although I used <span> around a bunch of Ordinary* abstract operation references, the build script is not complaining about missing <dfn>s. What is up with that?

In any event, this is definitely ready for review.

annevk · 2016-02-06T13:16:37Z

If tc39/ecma262#367 lands we need to figure out what to do with [[Enumerate]]. It's not entirely clear to me what we can do then.

domenic · 2016-02-06T14:53:35Z

Although I used around a bunch of Ordinary* abstract operation references, the build script is not complaining about missing s. What is up with that?

I've noticed this too. I think the build script thinks that empty spans are just meant to be using span to mark up a length of text, not auto-linking. That is why we should move to longer term.

Will try to review Monday.

sideshowbarker · 2016-02-08T11:02:50Z

Although I used around a bunch of Ordinary* abstract operation references, the build script is not complaining about missing s. What is up with that?

I've noticed this too. I think the build script thinks that empty spans are just meant to be using span to mark up a length of text, not auto-linking. That is why we should move to longer term.

To be clear, that’s an issue with wattsi, right? (Not something else in the build.sh script or the Perl scripts it calls.) If so, I wonder if whatwg/wattsi#5 regressed this, or if it’s always been this way, or what.

Ms2ger · 2016-02-08T14:57:48Z

source

+  <p>The <dfn>number of child browsing contexts</dfn> is the number of <span data-x="child browsing
+  context">child browsing contexts</span> that are <span data-x="browsing context nested
+  through">nested through</span> elements that are <span data-x="in a document">in the
+  <code>Document</code></span> that is the <span>active document</span> of the <code>Window</code>


A Window does not have an "active document"; a browsing context does.

annevk · 2016-02-10T12:52:40Z

WindowProxy needs to use https://heycam.github.io/webidl/#dfn-named-property-visibility (or a variant thereof) where it implements cross-origin named properties. In particular I think we don't want to have [OverrideBuiltins] behavior if I understood @bzbarsky's comments in https://bugzilla.mozilla.org/show_bug.cgi?id=1245554 correctly.

bzbarsky · 2016-02-10T14:59:10Z

We should probably test what UAs actually do with cross-origin named properties carefully. Defining how this should behave will be a bit tricky. For example, consider this testcase:

<!DOCTYPE html>
<body>
  <iframe name="foo"></iframe>
  <script>
    Object.prototype.foo = 5;
    onload = function() { alert(window.foo); }
  </script>
</body>

In browsers this alerts "5" (at least in Chrome, Safari, and Firefox). What should happen on cross-origin access to the "foo" property on this window? Chrome and Firefox return the subframe, looks like. Safari returns undefined (or 5, depending on what the accessing thing actually is; yes, it leaks info cross-origin in some cases). Of course maybe Safari returns undefined for cross-origin named access in general? We really need some exhaustive tests. :(

domenic · 2016-02-11T20:27:47Z

source

@@ -2976,16 +2977,23 @@ a.setAttribute('href', 'http://example.com/'); // change the content attribute d
     <li>The <dfn data-noexport="" data-x="js-prod-Module" data-x-href="https://tc39.github.io/ecma262/#prod-Module"><i>Module</i></dfn> production</li>
     <li>The <dfn data-noexport="" data-x="js-prod-Pattern" data-x-href="https://tc39.github.io/ecma262/#prod-Pattern"><i>Pattern</i></dfn> production</li>
     <li>The <dfn data-noexport="" data-x="js-prod-Script" data-x-href="https://tc39.github.io/ecma262/#prod-Script"><i>Script</i></dfn> production</li>
+     <li><dfn data-noexport="" data-x-href="https://tc39.github.io/ecma262/#sec-property-descriptor-specification-type">PropertyDescriptor</dfn></li>


I've been curating this section, perhaps a bit obsessively. This should be "The [PropertyDescriptor] specification type"

This is meant to refer to the tag literal descriptions of Property Descriptor records.

Yeah but ES conflates them

Still would like to see this fixed. The fact that ES uses specification type names as tags when creating literals is just notation; the actual thing being referenced is the PropertyDescriptor specification type.

Should it then say "Property Descriptor specification type"? But the references can use the literal PropertyDescriptor?

annevk · 2016-02-25T11:19:19Z

@domenic I had a few more issues opened against ECMAScript that are now resolved (two PRs still pending). The biggest change is the removal of [[HasProperty]] overrides due to an upstream change. Since OrdinaryHasProperty will invoke [[GetOwnProperty]] now instead of OrdinaryGetOwnProperty.

annevk · 2016-02-25T17:10:07Z

In another PR @domenic noted:

I think for these kind of informal maps, I much prefer the phrasing I used for the module map:

If module map contains an entry with key url, ...

If module map contains an entry with key url whose value is "fetching", ...

Create an entry in module map with key url and value "fetching"

Pretty sure we're not using that terminology for the internal map we are introducing here.

domenic · 2016-02-25T21:00:27Z

source

+   <li><p>Return false.</p></li>
+  </ol>
+
+  <h5 id="location-get-receiver">[[Get]] ( <var>P</var>, <var>Receiver</var> )</h5>


Can this be "location-get"?

annevk · 2016-02-26T09:10:22Z

Is the "do not merge yet" label there because of tc39/ecma262#416?

domenic · 2016-02-26T13:32:08Z

Yep

domenic · 2016-02-26T23:20:23Z

Dependencies merged; land at will @annevk!

This is a relative large change to the way Window, WindowProxy, and Location objects work, at least from a standards perspective. This should more closely match implementations and define all their relevant security details. A rough summary of the changes: * Window indexed getters have moved to WindowProxy. * WindowProxy is now a JavaScript exotic object and it handles all the security aspects for Window so Window can become an ordinary object. * Cross-origin named properties for Window are now exposed (though on WindowProxy). * Location is now an exotic object with all its internal methods overridden to handle the cross-origin security aspects. * Location no longer uses Unforgeable on the interface, allowing that to be removed from IDL. (It moved to all of its properties instead and the remaining details are defined through prose.) This should also address these bugs: * https://www.w3.org/Bugs/Public/show_bug.cgi?id=20701 * https://www.w3.org/Bugs/Public/show_bug.cgi?id=21674 * https://www.w3.org/Bugs/Public/show_bug.cgi?id=22346 * https://www.w3.org/Bugs/Public/show_bug.cgi?id=27128 * https://www.w3.org/Bugs/Public/show_bug.cgi?id=27502

sideshowbarker · 2016-02-27T07:04:43Z

Rock n rollーVery cool to see this landing.

domenic · 2016-02-27T15:24:24Z

This might deserve a blog post :D

Ms2ger · 2016-05-13T11:46:23Z

https://blog.whatwg.org/windowproxy-window-and-location

handonam · 2024-05-30T23:00:39Z

source

+  <p class="warning">The <code>Location</code> exotic object is defined through a mishmash of IDL,
+  invocation of JavaScript internal methods post-creation, and overridden JavaScript internal
+  methods. Coupled with its scary security policy, please take extra care while implementing
+  this excrescence.</p>


Wanted to say this is such an incredible word choice that had me search for over an hour on how this was added. Now i can relieve myself.

Ms2ger reviewed Feb 8, 2016
View reviewed changes

annevk mentioned this pull request Feb 9, 2016

Child browsing context names leak across origins #544

Closed

annevk assigned domenic Feb 10, 2016

annevk force-pushed the cross-origin-objects branch from 52a6d48 to 442575a Compare February 10, 2016 14:27

annevk mentioned this pull request Feb 11, 2016

Bugs to close when we move this to HTML annevk/html-cross-origin-objects#25

Closed

domenic reviewed Feb 11, 2016
View reviewed changes

domenic assigned annevk Feb 24, 2016

domenic added normative change addition/proposal New features or enhancements clarification Standard could be clearer labels Feb 24, 2016

annevk force-pushed the cross-origin-objects branch from 573c619 to ff6a7c1 Compare February 25, 2016 11:15

annevk assigned domenic and unassigned annevk Feb 25, 2016

annevk force-pushed the cross-origin-objects branch from 6f64898 to 7447524 Compare February 25, 2016 18:12

domenic reviewed Feb 25, 2016
View reviewed changes

domenic added the do not merge yet Pull request must not be merged per rationale in comment label Feb 25, 2016

annevk force-pushed the cross-origin-objects branch from 392492c to 918cd71 Compare February 26, 2016 09:04

domenic removed the do not merge yet Pull request must not be merged per rationale in comment label Feb 26, 2016

annevk force-pushed the cross-origin-objects branch from 918cd71 to acae3df Compare February 27, 2016 06:26

annevk merged commit acae3df into master Feb 27, 2016

annevk deleted the cross-origin-objects branch February 27, 2016 06:33

caitp mentioned this pull request Mar 9, 2016

Bug in test for cross-origin access to window[@@iterator] web-platform-tests/wpt#2664

Closed

lidel mentioned this pull request Nov 23, 2016

Not allowed to define a non-configurable property on the WindowProxy object sinonjs/sinon#1195

Closed

Skiddish mentioned this pull request Sep 23, 2021

Editorial: Introduce OrdinaryGet/Set/OwnPropertyKeys and use them tc39/ecma262#356

Closed

joyeecheung mentioned this pull request May 24, 2022

Access to private class members in the windows object is not checked #7952

Closed

handonam reviewed May 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Define security around Window, WindowProxy, and Location properly #638

Define security around Window, WindowProxy, and Location properly #638

annevk commented Feb 6, 2016

annevk commented Feb 6, 2016

annevk commented Feb 6, 2016

domenic commented Feb 6, 2016

sideshowbarker commented Feb 8, 2016

Ms2ger Feb 8, 2016

annevk commented Feb 10, 2016

bzbarsky commented Feb 10, 2016

domenic Feb 11, 2016

annevk Feb 12, 2016

domenic Feb 12, 2016

domenic Feb 12, 2016

annevk Feb 14, 2016

domenic Feb 14, 2016

annevk commented Feb 25, 2016

annevk commented Feb 25, 2016

domenic Feb 25, 2016

annevk commented Feb 26, 2016

domenic commented Feb 26, 2016

domenic commented Feb 26, 2016

sideshowbarker commented Feb 27, 2016

domenic commented Feb 27, 2016

Ms2ger commented May 13, 2016

handonam May 30, 2024

Define security around Window, WindowProxy, and Location properly #638

Define security around Window, WindowProxy, and Location properly #638

Conversation

annevk commented Feb 6, 2016

annevk commented Feb 6, 2016

annevk commented Feb 6, 2016

domenic commented Feb 6, 2016

sideshowbarker commented Feb 8, 2016

Choose a reason for hiding this comment

annevk commented Feb 10, 2016

bzbarsky commented Feb 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk commented Feb 25, 2016

annevk commented Feb 25, 2016

Choose a reason for hiding this comment

annevk commented Feb 26, 2016

domenic commented Feb 26, 2016

domenic commented Feb 26, 2016

sideshowbarker commented Feb 27, 2016

domenic commented Feb 27, 2016

Ms2ger commented May 13, 2016

Choose a reason for hiding this comment