iframe/frame/object.contentDocument should be on the prototype
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Feb 2016 03:27:44 +0000 (03:27 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Feb 2016 03:27:44 +0000 (03:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=154409

Reviewed by Sam Weinig.

LayoutTests/imported/w3c:

Rebaseline now that more checks are passing.

* web-platform-tests/html/dom/interfaces-expected.txt:

Source/WebCore:

Move iframe/frame/object.contentDocument to the prototype. They used
to be on the instance due to the [CheckSecurityForNode] IDL extended
attribute. This patch updates the bindings generator so that such
attributes are now on the prototype. While they are now on the
prototype, the security checks are still generated in the
corresponding getters and setters so cross origin access is still
prevented.

Test: http/tests/security/cross-origin-iframe-contentDocument.html

* bindings/scripts/CodeGeneratorJS.pm:
(AttributeShouldBeOnInstance): Deleted.

LayoutTests:

Add test coverage for trying to access iframe.contentDocument cross origin
to make sure it still fails and logs a security error.

* http/tests/security/cross-origin-iframe-contentDocument-expected.txt: Added.
* http/tests/security/cross-origin-iframe-contentDocument.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@196889 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/security/cross-origin-iframe-contentDocument-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/cross-origin-iframe-contentDocument.html [new file with mode: 0644]
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/html/dom/interfaces-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp

index bc3aa06..108d142 100644 (file)
@@ -1,3 +1,16 @@
+2016-02-21  Chris Dumez  <cdumez@apple.com>
+
+        iframe/frame/object.contentDocument should be on the prototype
+        https://bugs.webkit.org/show_bug.cgi?id=154409
+
+        Reviewed by Sam Weinig.
+
+        Add test coverage for trying to access iframe.contentDocument cross origin
+        to make sure it still fails and logs a security error.
+
+        * http/tests/security/cross-origin-iframe-contentDocument-expected.txt: Added.
+        * http/tests/security/cross-origin-iframe-contentDocument.html: Added.
+
 2016-02-21  Daniel Bates  <dabates@apple.com>
 
         CSP: sandbox directive should be ignored when contained in a policy defined via a meta element
diff --git a/LayoutTests/http/tests/security/cross-origin-iframe-contentDocument-expected.txt b/LayoutTests/http/tests/security/cross-origin-iframe-contentDocument-expected.txt
new file mode 100644 (file)
index 0000000..aa601cd
--- /dev/null
@@ -0,0 +1,16 @@
+CONSOLE MESSAGE: line 16: 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 1: 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 1: 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.
+Tests that iframe.contentDocument is not accessible cross-origin.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS crossOriginFrame.contentDocument returned null.
+PASS Object.getOwnPropertyDescriptor(sameOriginFrame.__proto__, "contentDocument").get.call(crossOriginFrame) returned null.
+PASS Object.getOwnPropertyDescriptor(crossOriginFrame.__proto__, "contentDocument").get.call(crossOriginFrame) returned null.
+PASS Object.getOwnPropertyDescriptor(crossOriginFrame, "contentDocument") returned undefined.
+PASS successfullyParsed is true
+
+TEST COMPLETE
diff --git a/LayoutTests/http/tests/security/cross-origin-iframe-contentDocument.html b/LayoutTests/http/tests/security/cross-origin-iframe-contentDocument.html
new file mode 100644 (file)
index 0000000..861f125
--- /dev/null
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body onload="runTest()">
+<iframe id="crossOriginFrame" src="http://localhost:8000/security/resources/blank.html"></iframe>
+<iframe id="sameOriginFrame" src="about:blank"></iframe>
+<script>
+description("Tests that iframe.contentDocument is not accessible cross-origin.");
+jsTestIsAsync = true;
+
+function shouldThrowOrReturnUndefinedOrNull(expression)
+{
+    try {
+        result = eval(expression);
+    } catch (e) {
+        testPassed(expression + " threw exception " + e + ".");
+        return;
+    }
+    if (result === undefined)
+        testPassed(expression + " returned undefined.");
+    else if (result === null)
+        testPassed(expression + " returned null.");
+    else
+        testFailed(expression + " returned " + result);
+}
+
+function runTest()
+{
+    crossOriginFrame = document.getElementById("crossOriginFrame");
+    sameOriginFrame = document.getElementById("sameOriginFrame");
+
+    shouldThrowOrReturnUndefinedOrNull('crossOriginFrame.contentDocument');
+    shouldThrowOrReturnUndefinedOrNull('Object.getOwnPropertyDescriptor(sameOriginFrame.__proto__, "contentDocument").get.call(crossOriginFrame)');
+    shouldThrowOrReturnUndefinedOrNull('Object.getOwnPropertyDescriptor(crossOriginFrame.__proto__, "contentDocument").get.call(crossOriginFrame)');
+    shouldThrowOrReturnUndefinedOrNull('Object.getOwnPropertyDescriptor(crossOriginFrame, "contentDocument")');
+    finishJSTest();
+}
+</script>
+</body>
+<script src="../../resources/js-test-post.js"></script>
+</html>
index 4383494..d998e92 100644 (file)
@@ -1,5 +1,16 @@
 2016-02-21  Chris Dumez  <cdumez@apple.com>
 
+        iframe/frame/object.contentDocument should be on the prototype
+        https://bugs.webkit.org/show_bug.cgi?id=154409
+
+        Reviewed by Sam Weinig.
+
+        Rebaseline now that more checks are passing.
+
+        * web-platform-tests/html/dom/interfaces-expected.txt:
+
+2016-02-21  Chris Dumez  <cdumez@apple.com>
+
         Re-sync W3C HTML/DOM web-platform-tests
         https://bugs.webkit.org/show_bug.cgi?id=154513
 
index e0378d7..9ca8fff 100644 (file)
@@ -1779,7 +1779,7 @@ FAIL HTMLIFrameElement interface: attribute seamless assert_true: The prototype
 FAIL HTMLIFrameElement interface: attribute allowFullscreen assert_true: The prototype object must have a property "allowFullscreen" expected true got false
 PASS HTMLIFrameElement interface: attribute width 
 PASS HTMLIFrameElement interface: attribute height 
-FAIL HTMLIFrameElement interface: attribute contentDocument assert_true: The prototype object must have a property "contentDocument" expected true got false
+PASS HTMLIFrameElement interface: attribute contentDocument 
 PASS HTMLIFrameElement interface: attribute contentWindow 
 PASS HTMLIFrameElement interface: attribute align 
 PASS HTMLIFrameElement interface: attribute scrolling 
@@ -1821,7 +1821,7 @@ PASS HTMLObjectElement interface: attribute useMap
 PASS HTMLObjectElement interface: attribute form 
 PASS HTMLObjectElement interface: attribute width 
 PASS HTMLObjectElement interface: attribute height 
-FAIL HTMLObjectElement interface: attribute contentDocument assert_true: The prototype object must have a property "contentDocument" expected true got false
+PASS HTMLObjectElement interface: attribute contentDocument 
 FAIL HTMLObjectElement interface: attribute contentWindow assert_true: The prototype object must have a property "contentWindow" expected true got false
 PASS HTMLObjectElement interface: attribute willValidate 
 PASS HTMLObjectElement interface: attribute validity 
@@ -4752,7 +4752,7 @@ PASS HTMLFrameElement interface: attribute src
 PASS HTMLFrameElement interface: attribute frameBorder 
 PASS HTMLFrameElement interface: attribute longDesc 
 PASS HTMLFrameElement interface: attribute noResize 
-FAIL HTMLFrameElement interface: attribute contentDocument assert_true: The prototype object must have a property "contentDocument" expected true got false
+PASS HTMLFrameElement interface: attribute contentDocument 
 PASS HTMLFrameElement interface: attribute contentWindow 
 PASS HTMLFrameElement interface: attribute marginHeight 
 PASS HTMLFrameElement interface: attribute marginWidth 
@@ -4764,7 +4764,7 @@ PASS HTMLFrameElement interface: document.createElement("frame") must inherit pr
 PASS HTMLFrameElement interface: document.createElement("frame") must inherit property "frameBorder" with the proper type (3) 
 PASS HTMLFrameElement interface: document.createElement("frame") must inherit property "longDesc" with the proper type (4) 
 PASS HTMLFrameElement interface: document.createElement("frame") must inherit property "noResize" with the proper type (5) 
-FAIL HTMLFrameElement interface: document.createElement("frame") must inherit property "contentDocument" with the proper type (6) assert_inherits: property "contentDocument" found on object expected in prototype chain
+PASS HTMLFrameElement interface: document.createElement("frame") must inherit property "contentDocument" with the proper type (6) 
 PASS HTMLFrameElement interface: document.createElement("frame") must inherit property "contentWindow" with the proper type (7) 
 PASS HTMLFrameElement interface: document.createElement("frame") must inherit property "marginHeight" with the proper type (8) 
 PASS HTMLFrameElement interface: document.createElement("frame") must inherit property "marginWidth" with the proper type (9) 
index 568c51c..920186e 100644 (file)
@@ -1,3 +1,23 @@
+2016-02-21  Chris Dumez  <cdumez@apple.com>
+
+        iframe/frame/object.contentDocument should be on the prototype
+        https://bugs.webkit.org/show_bug.cgi?id=154409
+
+        Reviewed by Sam Weinig.
+
+        Move iframe/frame/object.contentDocument to the prototype. They used
+        to be on the instance due to the [CheckSecurityForNode] IDL extended
+        attribute. This patch updates the bindings generator so that such
+        attributes are now on the prototype. While they are now on the
+        prototype, the security checks are still generated in the
+        corresponding getters and setters so cross origin access is still
+        prevented.
+
+        Test: http/tests/security/cross-origin-iframe-contentDocument.html
+
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (AttributeShouldBeOnInstance): Deleted.
+
 2016-02-21  Darin Adler  <darin@apple.com>
 
         Refactor LazyEventListener creation to separate Element and Document cases
index 07cbb0b..f506f00 100644 (file)
@@ -701,11 +701,6 @@ sub AttributeShouldBeOnInstance
     # https://heycam.github.io/webidl/#Unforgeable
     return 1 if IsUnforgeable($interface, $attribute);
 
-    # It becomes hard to reason about attributes that require security checks if we push
-    # them down the prototype chain, so before we do these we'll need to carefully consider
-    # the possible pitfalls.
-    return 1 if $attribute->signature->extendedAttributes->{"CheckSecurityForNode"};
-
     return 1 if AttributeShouldBeOnInstanceForCompatibility($interface, $attribute);
 
     if ($interface->extendedAttributes->{"CheckSecurity"}) {
index 85694e1..3a66d6b 100644 (file)
@@ -367,9 +367,9 @@ typedef JSDOMConstructor<JSTestObj> JSTestObjConstructor;
 /* Hash table */
 
 static const struct CompactHashIndex JSTestObjTableIndex[17] = {
-    { 5, -1 },
     { -1, -1 },
-    { 6, -1 },
+    { -1, -1 },
+    { 5, -1 },
     { -1, -1 },
     { -1, -1 },
     { -1, -1 },
@@ -406,11 +406,10 @@ static const HashTableValue JSTestObjTableValues[] =
 #else
     { 0, 0, NoIntrinsic, { 0, 0 } },
 #endif
-    { "contentDocument", ReadOnly | CustomAccessor, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjContentDocument), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(0) } },
     { "unforgeableMethod", DontDelete | ReadOnly | JSC::Function, NoIntrinsic, { (intptr_t)static_cast<NativeFunction>(jsTestObjInstanceFunctionUnforgeableMethod), (intptr_t) (0) } },
 };
 
-static const HashTable JSTestObjTable = { 7, 15, true, JSTestObjTableValues, JSTestObjTableIndex };
+static const HashTable JSTestObjTable = { 6, 15, true, JSTestObjTableValues, JSTestObjTableIndex };
 /* Hash table for constructor */
 
 static const HashTableValue JSTestObjConstructorTableValues[] =
@@ -574,6 +573,7 @@ static const HashTableValue JSTestObjPrototypeTableValues[] =
     { "cachedAttribute1", ReadOnly | CustomAccessor, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjCachedAttribute1), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(0) } },
     { "cachedAttribute2", ReadOnly | CustomAccessor, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjCachedAttribute2), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(0) } },
     { "anyAttribute", CustomAccessor, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjAnyAttribute), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestObjAnyAttribute) } },
+    { "contentDocument", ReadOnly | CustomAccessor, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjContentDocument), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(0) } },
     { "mutablePoint", CustomAccessor, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjMutablePoint), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestObjMutablePoint) } },
     { "immutablePoint", CustomAccessor, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjImmutablePoint), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestObjImmutablePoint) } },
     { "strawberry", CustomAccessor, NoIntrinsic, { (intptr_t)static_cast<PropertySlot::GetValueFunc>(jsTestObjStrawberry), (intptr_t) static_cast<PutPropertySlot::PutValueFunc>(setJSTestObjStrawberry) } },