Tweak the exposure of cross-origin properties by domenic · Pull Request #2777 · whatwg/html (original) (raw)
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.
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
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:
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
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.
annevk removed the needs tests
Moving the issue forward requires someone to write tests
label
OK cool, so this should be ready to merge. @annevk, will let you review/merge alongside #2777.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
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.
annevk deleted the cross-origin-object-tweaks branch
hubot pushed a commit to WebKit/WebKit-http that referenced this pull request
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:
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
The potential problem area would be the former change
Yeah, that part doesn't seem to be web-compatible. See #3183
ryanhaddad pushed a commit to WebKit/WebKit that referenced this pull request
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:
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
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:
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