Tweak the exposure of cross-origin properties #2777
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This contains two separate changes: * It makes all cross-origin properties that would normally be enumerable on same-origin objects, enumerable also on WindowProxy and Location objects (including when accessed same-origin). This includes safelisted methods and attributes, browsing context name properties, and browsing context index properties. The motivation for making them non-enumerable seems to have been a mistaken impression that doing so would prevent a cross-origin information leak. * It hides window names from [[OwnPropertyKeys]](), and thus Object.keys(), Object.getOwnPropertyNames(), etc. This actually prevents a cross-origin information leak. Closes #2753.
hubot
pushed a commit
to WebKit/WebKit-http
that referenced
this pull request
Jul 11, 2017
https://bugs.webkit.org/show_bug.cgi?id=174364 <rdar://problem/33238056> Reviewed by Brent Fulgham. Source/WebCore: Window's [[OwnPropertyKeys]] should not list descendant frame names when the window is cross-origin: - whatwg/html#2777 This aligns our behavior with Firefox and Chrome. No new tests, updated existing test. * bindings/js/JSDOMWindowCustom.cpp: (WebCore::addCrossOriginPropertyNames): (WebCore::addCrossOriginOwnPropertyNames): (WebCore::JSDOMWindow::getOwnPropertyNames): LayoutTests: Update test to reflect behavior change. I verified that the test is passing in Firefox. The test fails in Chrome because its does not expose frames indexes on the Window, and it is incorrectly listing "assign" on Location. * http/tests/security/cross-frame-access-enumeration.html: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@219355 268f45cc-cd09-0410-ab3c-d52691b4dbfc
annevk
added a commit
to web-platform-tests/wpt
that referenced
this pull request
Jul 13, 2017
Member
|
The latter change is web-compatible (and is actually what implementations do). Tests at web-platform-tests/wpt#6538. The potential problem area would be the former change I think, but maybe it's not a big deal. |
Member
Author
annevk
approved these changes
Jul 15, 2017
Member
annevk
left a comment
There was a problem hiding this comment.
Oh forgot to explicitly mark this as reviewed. I can land and file bugs on Monday.
annevk
added a commit
to web-platform-tests/wpt
that referenced
this pull request
Jul 17, 2017
And also, test that IDL so-called named properties are not exposed across origins (via [[OwnPropertyKeys]]()). See whatwg/html#2777 for the corresponding HTML Standard change.
hubot
pushed a commit
to WebKit/WebKit-http
that referenced
this pull request
Jul 19, 2017
https://bugs.webkit.org/show_bug.cgi?id=174576 Reviewed by Darin Adler. LayoutTests/imported/w3c: Re-sync tests from upstream and rebaseline to improve test coverage. * web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt: * web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html: * web-platform-tests/html/browsers/the-window-object/window-indexed-properties-expected.txt: * web-platform-tests/html/browsers/the-window-object/window-indexed-properties.html: Source/WebCore: Makes cross-origin properties enumerable on Window and Location objects as per: - whatwg/html#2777 This simplifies our code quite a bit. No new tests, updated existing tests. * bindings/js/JSDOMWindowCustom.cpp: (WebCore::jsDOMWindowGetOwnPropertySlotRestrictedAccess): (WebCore::JSDOMWindow::getOwnPropertySlotByIndex): (WebCore::JSDOMWindow::getOwnPropertyNames): * bindings/js/JSLocationCustom.cpp: (WebCore::getOwnPropertySlotCommon): (WebCore::JSLocation::getOwnPropertyNames): * bindings/scripts/CodeGeneratorJS.pm: (GenerateHeader): LayoutTests: Update / rebaseline some tests to reflect behavior change. * http/tests/security/cross-origin-descriptors-expected.txt: * http/tests/security/cross-origin-descriptors.html: * js/dom/getOwnPropertyDescriptor-expected.txt: * js/resources/getOwnPropertyDescriptor.js: git-svn-id: http://svn.webkit.org/repository/webkit/trunk@219659 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Contributor
Yeah, that part doesn't seem to be web-compatible. See #3183 |
ryanhaddad
pushed a commit
to WebKit/WebKit
that referenced
this pull request
Dec 22, 2020
https://bugs.webkit.org/show_bug.cgi?id=174364 <rdar://problem/33238056> Reviewed by Brent Fulgham. Source/WebCore: Window's [[OwnPropertyKeys]] should not list descendant frame names when the window is cross-origin: - whatwg/html#2777 This aligns our behavior with Firefox and Chrome. No new tests, updated existing test. * bindings/js/JSDOMWindowCustom.cpp: (WebCore::addCrossOriginPropertyNames): (WebCore::addCrossOriginOwnPropertyNames): (WebCore::JSDOMWindow::getOwnPropertyNames): LayoutTests: Update test to reflect behavior change. I verified that the test is passing in Firefox. The test fails in Chrome because its does not expose frames indexes on the Window, and it is incorrectly listing "assign" on Location. * http/tests/security/cross-frame-access-enumeration.html: Canonical link: https://commits.webkit.org/191196@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219355 268f45cc-cd09-0410-ab3c-d52691b4dbfc
ryanhaddad
pushed a commit
to WebKit/WebKit
that referenced
this pull request
Dec 22, 2020
https://bugs.webkit.org/show_bug.cgi?id=174576 Reviewed by Darin Adler. LayoutTests/imported/w3c: Re-sync tests from upstream and rebaseline to improve test coverage. * web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects-expected.txt: * web-platform-tests/html/browsers/origin/cross-origin-objects/cross-origin-objects.html: * web-platform-tests/html/browsers/the-window-object/window-indexed-properties-expected.txt: * web-platform-tests/html/browsers/the-window-object/window-indexed-properties.html: Source/WebCore: Makes cross-origin properties enumerable on Window and Location objects as per: - whatwg/html#2777 This simplifies our code quite a bit. No new tests, updated existing tests. * bindings/js/JSDOMWindowCustom.cpp: (WebCore::jsDOMWindowGetOwnPropertySlotRestrictedAccess): (WebCore::JSDOMWindow::getOwnPropertySlotByIndex): (WebCore::JSDOMWindow::getOwnPropertyNames): * bindings/js/JSLocationCustom.cpp: (WebCore::getOwnPropertySlotCommon): (WebCore::JSLocation::getOwnPropertyNames): * bindings/scripts/CodeGeneratorJS.pm: (GenerateHeader): LayoutTests: Update / rebaseline some tests to reflect behavior change. * http/tests/security/cross-origin-descriptors-expected.txt: * http/tests/security/cross-origin-descriptors.html: * js/dom/getOwnPropertyDescriptor-expected.txt: * js/resources/getOwnPropertyDescriptor.js: Canonical link: https://commits.webkit.org/191470@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219659 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This contains two separate changes:
It makes all cross-origin properties that would normally be enumerable
on same-origin objects, enumerable also on WindowProxy and Location
objects (including when accessed same-origin). This includes
safelisted methods and attributes, browsing context name properties,
and browsing context index properties. The motivation for making them
non-enumerable seems to have been a mistaken impression that doing so
would prevent a cross-origin information leak.
It hides window names from [[OwnPropertyKeys]](), and thus
Object.keys(), Object.getOwnPropertyNames(), etc. This actually
prevents a cross-origin information leak.
Closes #2753.
Marking "do not merge yet" until we are aware of the web-compatibility implications of the latter change. Writing tests should help with that.