- Revision
- 205154
- Author
- cdu...@apple.com
- Date
- 2016-08-29 16:04:44 -0700 (Mon, 29 Aug 2016)
Log Message
Regression(r204923): It should be possible to set 'Location.href' cross origin
https://bugs.webkit.org/show_bug.cgi?id=161343
<rdar://problem/28063361>
Reviewed by Ryosuke Niwa.
Source/WebCore:
It should be possible to set 'Location.href' cross origin:
- https://html.spec.whatwg.org/#crossoriginproperties-(-o-)
Firefox and Chrome allow this but we throw a SecurityError.
We already allow setting crossOrigin.window.location which is equivalent.
No new tests, updated existing test.
* bindings/js/JSLocationCustom.cpp:
(WebCore::JSLocation::putDelegate):
Refactor the [Put] delegate so that it does not log a security error
when setting 'href' attribute, given that setting it works as expected.
This fixes a bug in shipping Safari where setting 'href' would work but
log an error message anyway.
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateImplementation):
Add support for [DoNotCheckSecurityOnSetter] IDL extended attribute,
in addition to the already supported [DoNotCheckSecurity] and
[DoNotCheckSecurityOnGetter].
* page/Location.idl:
Use [DoNotCheckSecurityOnSetter] on 'href' attribute as it can be
set cross-origin. This fixes the regression introduced in r204923.
LayoutTests:
Add layout test coverage.
* http/tests/security/location-cross-origin-expected.txt:
* http/tests/security/location-cross-origin.html:
* http/tests/security/xss-DENIED-assign-location-href-_javascript_-expected.txt:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (205153 => 205154)
--- trunk/LayoutTests/ChangeLog 2016-08-29 22:54:48 UTC (rev 205153)
+++ trunk/LayoutTests/ChangeLog 2016-08-29 23:04:44 UTC (rev 205154)
@@ -1,3 +1,17 @@
+2016-08-29 Chris Dumez <cdu...@apple.com>
+
+ Regression(r204923): It should be possible to set 'Location.href' cross origin
+ https://bugs.webkit.org/show_bug.cgi?id=161343
+ <rdar://problem/28063361>
+
+ Reviewed by Ryosuke Niwa.
+
+ Add layout test coverage.
+
+ * http/tests/security/location-cross-origin-expected.txt:
+ * http/tests/security/location-cross-origin.html:
+ * http/tests/security/xss-DENIED-assign-location-href-_javascript_-expected.txt:
+
2016-08-29 Jiewen Tan <jiewen_...@apple.com>
Unreviewed, update iOS simulator WK1 flaky tests.
Modified: trunk/LayoutTests/http/tests/security/location-cross-origin-expected.txt (205153 => 205154)
--- trunk/LayoutTests/http/tests/security/location-cross-origin-expected.txt 2016-08-29 22:54:48 UTC (rev 205153)
+++ trunk/LayoutTests/http/tests/security/location-cross-origin-expected.txt 2016-08-29 23:04:44 UTC (rev 205154)
@@ -27,6 +27,8 @@
PASS Object.getOwnPropertyDescriptor(window.location, 'ancestorOrigins').get.call(frames[0].location) threw exception SecurityError (DOM Exception 18): Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match..
PASS Object.getOwnPropertyDescriptor(window.location, 'toString').value.call(frames[0].location) threw exception SecurityError (DOM Exception 18): Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match..
PASS Object.getOwnPropertyDescriptor(window.location, 'href').get.call(frames[0].location) threw exception SecurityError (DOM Exception 18): Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match..
+PASS frames[0].location.href = '' did not throw exception.
+PASS frames[0].location.href is "about:blank"
PASS successfullyParsed is true
TEST COMPLETE
Modified: trunk/LayoutTests/http/tests/security/location-cross-origin.html (205153 => 205154)
--- trunk/LayoutTests/http/tests/security/location-cross-origin.html 2016-08-29 22:54:48 UTC (rev 205153)
+++ trunk/LayoutTests/http/tests/security/location-cross-origin.html 2016-08-29 23:04:44 UTC (rev 205154)
@@ -34,7 +34,12 @@
shouldThrowErrorName("Object.getOwnPropertyDescriptor(window.location, 'toString').value.call(frames[0].location)", "SecurityError");
shouldThrowErrorName("Object.getOwnPropertyDescriptor(window.location, 'href').get.call(frames[0].location)", "SecurityError");
- finishJSTest();
+ // Setting 'href' cross origin should be allowed.
+ shouldNotThrow("frames[0].location.href = ''");
+ setTimeout(function() {
+ shouldBeEqualToString("frames[0].location.href", "about:blank");
+ finishJSTest();
+ }, 100);
};
</script>
<script src=""
Modified: trunk/LayoutTests/http/tests/security/xss-DENIED-assign-location-href-_javascript_-expected.txt (205153 => 205154)
--- trunk/LayoutTests/http/tests/security/xss-DENIED-assign-location-href-_javascript_-expected.txt 2016-08-29 22:54:48 UTC (rev 205153)
+++ trunk/LayoutTests/http/tests/security/xss-DENIED-assign-location-href-_javascript_-expected.txt 2016-08-29 23:04:44 UTC (rev 205154)
@@ -1,5 +1,4 @@
CONSOLE MESSAGE: line 13: Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match.
-CONSOLE MESSAGE: line 9: SecurityError (DOM Exception 18): Blocked a frame with origin "http://127.0.0.1:8000" from accessing a frame with origin "http://localhost:8000". Protocols, domains, and ports must match.
--------
Modified: trunk/Source/WebCore/ChangeLog (205153 => 205154)
--- trunk/Source/WebCore/ChangeLog 2016-08-29 22:54:48 UTC (rev 205153)
+++ trunk/Source/WebCore/ChangeLog 2016-08-29 23:04:44 UTC (rev 205154)
@@ -1,5 +1,39 @@
2016-08-29 Chris Dumez <cdu...@apple.com>
+ Regression(r204923): It should be possible to set 'Location.href' cross origin
+ https://bugs.webkit.org/show_bug.cgi?id=161343
+ <rdar://problem/28063361>
+
+ Reviewed by Ryosuke Niwa.
+
+ It should be possible to set 'Location.href' cross origin:
+ - https://html.spec.whatwg.org/#crossoriginproperties-(-o-)
+
+ Firefox and Chrome allow this but we throw a SecurityError.
+
+ We already allow setting crossOrigin.window.location which is equivalent.
+
+ No new tests, updated existing test.
+
+ * bindings/js/JSLocationCustom.cpp:
+ (WebCore::JSLocation::putDelegate):
+ Refactor the [Put] delegate so that it does not log a security error
+ when setting 'href' attribute, given that setting it works as expected.
+ This fixes a bug in shipping Safari where setting 'href' would work but
+ log an error message anyway.
+
+ * bindings/scripts/CodeGeneratorJS.pm:
+ (GenerateImplementation):
+ Add support for [DoNotCheckSecurityOnSetter] IDL extended attribute,
+ in addition to the already supported [DoNotCheckSecurity] and
+ [DoNotCheckSecurityOnGetter].
+
+ * page/Location.idl:
+ Use [DoNotCheckSecurityOnSetter] on 'href' attribute as it can be
+ set cross-origin. This fixes the regression introduced in r204923.
+
+2016-08-29 Chris Dumez <cdu...@apple.com>
+
We should throw a SecurityError when denying setting a cross-origin Window property
https://bugs.webkit.org/show_bug.cgi?id=161339
Modified: trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp (205153 => 205154)
--- trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp 2016-08-29 22:54:48 UTC (rev 205153)
+++ trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp 2016-08-29 23:04:44 UTC (rev 205154)
@@ -70,13 +70,19 @@
if (propertyName == exec->propertyNames().toString || propertyName == exec->propertyNames().valueOf)
return true;
- if (shouldAllowAccessToFrame(exec, frame))
+ String errorMessage;
+ if (shouldAllowAccessToFrame(exec, frame, errorMessage))
return false;
// Cross-domain access to the location is allowed when assigning the whole location,
- //but not when assigning the individual pieces, since that might inadvertently
+ // but not when assigning the individual pieces, since that might inadvertently
// disclose other parts of the original location.
- return propertyName != exec->propertyNames().href;
+ if (propertyName != exec->propertyNames().href) {
+ // FIXME: We should throw a SecurityError.
+ printErrorMessageForFrame(frame, errorMessage);
+ return true;
+ }
+ return false;
}
bool JSLocation::deleteProperty(JSCell* cell, ExecState* exec, PropertyName propertyName)
Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (205153 => 205154)
--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2016-08-29 22:54:48 UTC (rev 205153)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm 2016-08-29 23:04:44 UTC (rev 205154)
@@ -3125,7 +3125,7 @@
}
push(@implContent, " }\n");
}
- if ($interface->extendedAttributes->{"CheckSecurity"} && !$attribute->signature->extendedAttributes->{"DoNotCheckSecurity"}) {
+ if ($interface->extendedAttributes->{"CheckSecurity"} && !$attribute->signature->extendedAttributes->{"DoNotCheckSecurity"} && !$attribute->signature->extendedAttributes->{"DoNotCheckSecurityOnSetter"}) {
if ($interfaceName eq "DOMWindow") {
push(@implContent, " if (!BindingSecurity::shouldAllowAccessToDOMWindow(state, castedThis->wrapped(), ThrowSecurityError))\n");
} else {
Modified: trunk/Source/WebCore/page/Location.idl (205153 => 205154)
--- trunk/Source/WebCore/page/Location.idl 2016-08-29 22:54:48 UTC (rev 205153)
+++ trunk/Source/WebCore/page/Location.idl 2016-08-29 23:04:44 UTC (rev 205154)
@@ -38,7 +38,7 @@
JSCustomNamedGetterOnPrototype,
Unforgeable,
] interface Location {
- [SetterCallWith=ActiveWindow&FirstWindow] stringifier attribute USVString href;
+ [SetterCallWith=ActiveWindow&FirstWindow, DoNotCheckSecurityOnSetter] stringifier attribute USVString href;
[CallWith=ActiveWindow&FirstWindow, ForwardDeclareInHeader] void assign(USVString url);
[DoNotCheckSecurity, CallWith=ActiveWindow&FirstWindow, ForwardDeclareInHeader] void replace(USVString url);
