Title: [91957] trunk
Revision
91957
Author
aba...@webkit.org
Date
2011-07-28 16:39:27 -0700 (Thu, 28 Jul 2011)

Log Message

Old code about empty security origins could use a bath
https://bugs.webkit.org/show_bug.cgi?id=64735

Reviewed by Dimitri Glazkov.

Source/WebCore:

This patch cleans up some old code related to empty security origins.
It also removes some dodgy code that seems wrong.

Test: http/tests/security/inactive-document-with-empty-security-origin.html

* bindings/generic/BindingSecurityBase.cpp:
(WebCore::BindingSecurityBase::canAccess):
* page/SecurityOrigin.cpp:
(WebCore::SecurityOrigin::isEmpty):
(WebCore::SecurityOrigin::isSecureTransitionTo):
(WebCore::SecurityOrigin::toString):

LayoutTests:

* http/tests/security/inactive-document-with-empty-security-origin-expected.txt: Added.
* http/tests/security/inactive-document-with-empty-security-origin.html: Added.
* http/tests/security/resources/post-done-to-opener.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (91956 => 91957)


--- trunk/LayoutTests/ChangeLog	2011-07-28 23:37:46 UTC (rev 91956)
+++ trunk/LayoutTests/ChangeLog	2011-07-28 23:39:27 UTC (rev 91957)
@@ -1,3 +1,14 @@
+2011-07-28  Adam Barth  <aba...@webkit.org>
+
+        Old code about empty security origins could use a bath
+        https://bugs.webkit.org/show_bug.cgi?id=64735
+
+        Reviewed by Dimitri Glazkov.
+
+        * http/tests/security/inactive-document-with-empty-security-origin-expected.txt: Added.
+        * http/tests/security/inactive-document-with-empty-security-origin.html: Added.
+        * http/tests/security/resources/post-done-to-opener.html: Added.
+
 2011-07-28  Mihnea Ovidenie  <mih...@adobe.com>
 
         [CSSRegions]Add basic RenderRegion support

Added: trunk/LayoutTests/http/tests/security/inactive-document-with-empty-security-origin-expected.txt (0 => 91957)


--- trunk/LayoutTests/http/tests/security/inactive-document-with-empty-security-origin-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/inactive-document-with-empty-security-origin-expected.txt	2011-07-28 23:39:27 UTC (rev 91957)
@@ -0,0 +1 @@
+This test passes if it doesn't alert something ugly.

Added: trunk/LayoutTests/http/tests/security/inactive-document-with-empty-security-origin.html (0 => 91957)


--- trunk/LayoutTests/http/tests/security/inactive-document-with-empty-security-origin.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/inactive-document-with-empty-security-origin.html	2011-07-28 23:39:27 UTC (rev 91957)
@@ -0,0 +1,47 @@
+<script>
+(function() {
+
+if (location.hash == "#stop")
+    return;
+
+if (window.layoutTestController) {
+    layoutTestController.dumpAsText();
+    layoutTestController.waitUntilDone();
+    layoutTestController.setCanOpenWindows();
+    layoutTestController.setCloseRemainingWindowsWhenComplete(true);
+}
+
+blankWindow = open(location + "#stop");
+blankWindow._onload_ = function() {
+    blankFunc = blankWindow.Function;
+    blankWindow.opener = null;
+    blankWindow.eval("location = 'about:blank'");
+
+    setTimeout(function() {
+        blankFunc = blankFunc("return window.Function")();
+        if (!blankFunc && window.layoutTestController) {
+            layoutTestController.notifyDone()
+            return;
+        }
+        blankFunc("alertFunc", "(" + function() {
+            targetWindow = open("http://localhost:8080/security/resources/post-done-to-opener.html");
+            targetFunc = targetWindow.Function;
+            this.alertFunc = alertFunc;
+
+            targetWindow.addEventListener("load", function() {
+                alertFunc(targetFunc("return document.documentElement.innerHTML")());
+            });
+        } + ")()")( function(s) { alert(s) });
+    }, 0);
+}
+
+window.addEventListener('message', function(evt) {
+    if (evt.data == 'done') {
+        if (window.layoutTestController)
+            layoutTestController.notifyDone();
+    }
+});
+
+})();
+</script>
+This test passes if it doesn't alert something ugly.

Added: trunk/LayoutTests/http/tests/security/resources/post-done-to-opener.html (0 => 91957)


--- trunk/LayoutTests/http/tests/security/resources/post-done-to-opener.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/resources/post-done-to-opener.html	2011-07-28 23:39:27 UTC (rev 91957)
@@ -0,0 +1,4 @@
+<script>
+opener.postMessage('done', '*');
+</script>
+This frame sends a 'done' message to the opener window.

Added: trunk/LayoutTests/platform/chromium/http/tests/security/inactive-document-with-empty-security-origin-expected.txt (0 => 91957)


--- trunk/LayoutTests/platform/chromium/http/tests/security/inactive-document-with-empty-security-origin-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/chromium/http/tests/security/inactive-document-with-empty-security-origin-expected.txt	2011-07-28 23:39:27 UTC (rev 91957)
@@ -0,0 +1,3 @@
+CONSOLE MESSAGE: line 1: Unsafe _javascript_ attempt to access frame with URL about:blank from frame with URL http://127.0.0.1:8000/security/inactive-document-with-empty-security-origin.html. Domains, protocols and ports must match.
+
+This test passes if it doesn't alert something ugly.

Modified: trunk/Source/WebCore/ChangeLog (91956 => 91957)


--- trunk/Source/WebCore/ChangeLog	2011-07-28 23:37:46 UTC (rev 91956)
+++ trunk/Source/WebCore/ChangeLog	2011-07-28 23:39:27 UTC (rev 91957)
@@ -1,3 +1,22 @@
+2011-07-28  Adam Barth  <aba...@webkit.org>
+
+        Old code about empty security origins could use a bath
+        https://bugs.webkit.org/show_bug.cgi?id=64735
+
+        Reviewed by Dimitri Glazkov.
+
+        This patch cleans up some old code related to empty security origins.
+        It also removes some dodgy code that seems wrong.
+
+        Test: http/tests/security/inactive-document-with-empty-security-origin.html
+
+        * bindings/generic/BindingSecurityBase.cpp:
+        (WebCore::BindingSecurityBase::canAccess):
+        * page/SecurityOrigin.cpp:
+        (WebCore::SecurityOrigin::isEmpty):
+        (WebCore::SecurityOrigin::isSecureTransitionTo):
+        (WebCore::SecurityOrigin::toString):
+
 2011-07-28  Vsevolod Vlasov  <vse...@chromium.org>
 
         Web Inspector: [REGRESSION] Inspected tab crashes if navigated with inspector open and there are watch expressions added.

Modified: trunk/Source/WebCore/bindings/generic/BindingSecurityBase.cpp (91956 => 91957)


--- trunk/Source/WebCore/bindings/generic/BindingSecurityBase.cpp	2011-07-28 23:37:46 UTC (rev 91956)
+++ trunk/Source/WebCore/bindings/generic/BindingSecurityBase.cpp	2011-07-28 23:39:27 UTC (rev 91957)
@@ -48,47 +48,17 @@
     return node->document()->frame();
 }
 
-// Same origin policy implementation:
-//
-// Same origin policy prevents JS code from domain A from accessing JS & DOM
-// objects in a different domain B. There are exceptions and several objects
-// are accessible by cross-domain code. For example, the window.frames object
-// is accessible by code from a different domain, but window.document is not.
-//
-// The JS binding code sets security check callbacks on a function template,
-// and accessing instances of the template calls the callback function.
-// The callback function enforces the same origin policy.
-//
-// Callback functions are expensive. Binding code should use a security token
-// string to do fast access checks for the common case where source and target
-// are in the same domain. A security token is a string object that represents
-// the protocol/url/port of a domain.
-//
-// There are special cases where security token matching is not enough.
-// For example, JS can set its domain to a super domain by calling
-// document.setDomain(...). In these cases, the binding code can reset
-// a context's security token to its global object so that the fast access
-// check will always fail.
-
-// Helper to check if the current execution context can access a target frame.
-// First it checks same domain policy using the lexical context.
-//
-// This is equivalent to KJS::Window::allowsAccessFrom(ExecState*).
-bool BindingSecurityBase::canAccess(DOMWindow* activeWindow,
-                                    DOMWindow* targetWindow)
+bool BindingSecurityBase::canAccess(DOMWindow* activeWindow, DOMWindow* targetWindow)
 {
     ASSERT(targetWindow);
-
-    String message;
-
     if (activeWindow == targetWindow)
         return true;
 
     if (!activeWindow)
         return false;
 
-    const SecurityOrigin* activeSecurityOrigin = activeWindow->securityOrigin();
-    const SecurityOrigin* targetSecurityOrigin = targetWindow->securityOrigin();
+    SecurityOrigin* activeSecurityOrigin = activeWindow->securityOrigin();
+    SecurityOrigin* targetSecurityOrigin = targetWindow->securityOrigin();
 
     // We have seen crashes were the security origin of the target has not been
     // initialized. Defend against that.
@@ -98,12 +68,7 @@
     if (activeSecurityOrigin->canAccess(targetSecurityOrigin))
         return true;
 
-    // Allow access to a "about:blank" page if the dynamic context is a
-    // detached context of the same frame as the blank page.
-    if (targetSecurityOrigin->isEmpty() && activeWindow->frame() == targetWindow->frame())
-        return true;
-
     return false;
 }
 
-} // namespace WebCore
+}

Modified: trunk/Source/WebCore/page/SecurityOrigin.cpp (91956 => 91957)


--- trunk/Source/WebCore/page/SecurityOrigin.cpp	2011-07-28 23:37:46 UTC (rev 91956)
+++ trunk/Source/WebCore/page/SecurityOrigin.cpp	2011-07-28 23:39:27 UTC (rev 91957)
@@ -147,6 +147,7 @@
 
 bool SecurityOrigin::isEmpty() const
 {
+    ASSERT(!m_protocol.isEmpty() || m_isUnique);
     return m_protocol.isEmpty();
 }
 
@@ -173,6 +174,7 @@
     m_domain = newDomain.lower();
 }
 
+// FIXME: This should move to SchemeRegistry!
 static HashSet<String>& schemesForbiddenFromDomainRelaxation()
 {
     DEFINE_STATIC_LOCAL(HashSet<String>, schemes, ());
@@ -373,8 +375,8 @@
 }
 
 bool SecurityOrigin::isSecureTransitionTo(const KURL& url) const
-{ 
-    // New window created by the application
+{
+    // This origin represents a new window created by the application.
     if (isEmpty())
         return true;
 
@@ -384,9 +386,6 @@
 
 String SecurityOrigin::toString() const
 {
-    if (isEmpty())
-        return "null";
-
     if (isUnique())
         return "null";
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to