[V8] Re-wire "target" half of the same-origin security check through Document rather...
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 4 Aug 2012 07:31:41 +0000 (07:31 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 4 Aug 2012 07:31:41 +0000 (07:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=93079

Reviewed by Eric Seidel.

Before this patch, we were traversing from Nodes to Frames to
DOMWindows to SecurityOrigins when determing the "target" of an
operation for the same-origin policy security check. Rather than
detouring through DOMWindow, these security checks should operate in
terms of ScriptExecutionContexts (aka Documents) because that's the
canonical place we store SecurityOrigin objects.

A future patch will re-wire the "active" part of the security check to
use ScriptExecutionContexts as well and we'll be able to remove the
extra copy of SecurityOrigin that we keep in DOMWindow.

* bindings/generic/BindingSecurity.cpp:
(WebCore::canAccessDocument):
(WebCore::BindingSecurity::canAccessFrame):
(WebCore::BindingSecurity::shouldAllowAccessToNode):
* bindings/v8/BindingState.cpp:
(WebCore::immediatelyReportUnsafeAccessTo):
* bindings/v8/BindingState.h:
(WebCore):
* bindings/v8/V8DOMWindowShell.cpp:
(WebCore::reportUnsafeJavaScriptAccess):
* bindings/v8/V8Proxy.cpp:
(WebCore::V8Proxy::reportUnsafeAccessTo):
* bindings/v8/V8Proxy.h:
(V8Proxy):

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

Source/WebCore/ChangeLog
Source/WebCore/bindings/generic/BindingSecurity.cpp
Source/WebCore/bindings/v8/BindingState.cpp
Source/WebCore/bindings/v8/BindingState.h
Source/WebCore/bindings/v8/V8DOMWindowShell.cpp
Source/WebCore/bindings/v8/V8Proxy.cpp
Source/WebCore/bindings/v8/V8Proxy.h

index f7b2a21..3cb4c52 100644 (file)
@@ -1,3 +1,36 @@
+2012-08-04  Adam Barth  <abarth@webkit.org>
+
+        [V8] Re-wire "target" half of the same-origin security check through Document rather than DOMWindow
+        https://bugs.webkit.org/show_bug.cgi?id=93079
+
+        Reviewed by Eric Seidel.
+
+        Before this patch, we were traversing from Nodes to Frames to
+        DOMWindows to SecurityOrigins when determing the "target" of an
+        operation for the same-origin policy security check. Rather than
+        detouring through DOMWindow, these security checks should operate in
+        terms of ScriptExecutionContexts (aka Documents) because that's the
+        canonical place we store SecurityOrigin objects.
+
+        A future patch will re-wire the "active" part of the security check to
+        use ScriptExecutionContexts as well and we'll be able to remove the
+        extra copy of SecurityOrigin that we keep in DOMWindow.
+
+        * bindings/generic/BindingSecurity.cpp:
+        (WebCore::canAccessDocument):
+        (WebCore::BindingSecurity::canAccessFrame):
+        (WebCore::BindingSecurity::shouldAllowAccessToNode):
+        * bindings/v8/BindingState.cpp:
+        (WebCore::immediatelyReportUnsafeAccessTo):
+        * bindings/v8/BindingState.h:
+        (WebCore):
+        * bindings/v8/V8DOMWindowShell.cpp:
+        (WebCore::reportUnsafeJavaScriptAccess):
+        * bindings/v8/V8Proxy.cpp:
+        (WebCore::V8Proxy::reportUnsafeAccessTo):
+        * bindings/v8/V8Proxy.h:
+        (V8Proxy):
+
 2012-08-03  Adam Barth  <abarth@webkit.org>
 
         Disabling eval changes the timing of DidCreateScriptContext
index 1f37b79..a298cae 100644 (file)
 
 namespace WebCore {
 
-static bool canAccess(DOMWindow* activeWindow, DOMWindow* targetWindow)
+static bool canAccessDocument(BindingState* state, Document* targetDocument, bool reportError)
 {
-    ASSERT(targetWindow);
-    if (activeWindow == targetWindow)
-        return true;
-
-    if (!activeWindow)
+    // We have seen crashes were the target is 0, but we don't have a test case for it.
+    if (!targetDocument)
         return false;
 
-    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.
-    if (!targetSecurityOrigin)
+    DOMWindow* active = activeWindow(state);
+    if (!active)
         return false;
 
-    if (activeSecurityOrigin->canAccess(targetSecurityOrigin))
+    if (active->securityOrigin()->canAccess(targetDocument->securityOrigin()))
         return true;
 
+    if (reportError)
+        immediatelyReportUnsafeAccessTo(state, targetDocument);
+
     return false;
 }
 
 bool BindingSecurity::canAccessFrame(BindingState* state, Frame* target, bool reportError)
 {
-    if (!target)
-        return false;
-
-    if (!canAccess(activeWindow(state), target->domWindow())) {
-        if (reportError)
-            immediatelyReportUnsafeAccessTo(state, target);
-        return false;
-    }
-    return true;
+    return target && canAccessDocument(state, target->document(), reportError);
 }
 
 bool BindingSecurity::shouldAllowAccessToNode(BindingState* state, Node* node)
@@ -83,6 +71,7 @@ bool BindingSecurity::shouldAllowAccessToNode(BindingState* state, Node* node)
     if (!node)
         return false;
 
+    // FIXME: We shouldn't need to go through the frame here because we already have the document.
     Frame* target = node->document()->frame();
     if (!target)
         return false;
index c4d9787..fdf5c07 100644 (file)
@@ -89,9 +89,9 @@ Frame* currentFrame(BindingState*)
     return V8Proxy::retrieveFrame(context);
 }
 
-void immediatelyReportUnsafeAccessTo(BindingState*, Frame* target)
+void immediatelyReportUnsafeAccessTo(BindingState*, Document* targetDocument)
 {
-    V8Proxy::reportUnsafeAccessTo(target);
+    V8Proxy::reportUnsafeAccessTo(targetDocument);
 }
 
 }
index 7000803..9711ee8 100644 (file)
@@ -34,6 +34,7 @@
 namespace WebCore {
 
 class DOMWindow;
+class Document;
 class Frame;
 
 class BindingState {
@@ -53,7 +54,7 @@ Frame* firstFrame(BindingState*);
 // are any subtle differences between the currentFrame and the lexicalGlobalObject.
 Frame* currentFrame(BindingState*);
 
-void immediatelyReportUnsafeAccessTo(BindingState*, Frame*);
+void immediatelyReportUnsafeAccessTo(BindingState*, Document* targetDocument);
 
 }
 
index 03cecce..7558ae6 100644 (file)
@@ -153,7 +153,7 @@ static void reportUnsafeJavaScriptAccess(v8::Local<v8::Object> host, v8::AccessT
 {
     Frame* target = getTargetFrame(host, data);
     if (target)
-        V8Proxy::reportUnsafeAccessTo(target);
+        V8Proxy::reportUnsafeAccessTo(target->document());
 }
 
 PassRefPtr<V8DOMWindowShell> V8DOMWindowShell::create(Frame* frame)
index 681fc23..be696b5 100644 (file)
@@ -124,13 +124,12 @@ typedef HashMap<Node*, v8::Object*> DOMNodeMap;
 typedef HashMap<void*, v8::Object*> DOMObjectMap;
 typedef HashMap<int, v8::FunctionTemplate*> FunctionTemplateMap;
 
-void V8Proxy::reportUnsafeAccessTo(Frame* target)
+void V8Proxy::reportUnsafeAccessTo(Document* targetDocument)
 {
-    ASSERT(target);
-    Document* targetDocument = target->document();
     if (!targetDocument)
         return;
 
+    // FIXME: We should pass both the active and target documents in as arguments.
     Frame* source = firstFrame(BindingState::instance());
     if (!source)
         return;
index 478a643..f350564 100644 (file)
@@ -224,8 +224,7 @@ namespace WebCore {
 
         static const V8Extensions& extensions();
 
-        // Report an unsafe attempt to access the given frame on the console.
-        static void reportUnsafeAccessTo(Frame* target);
+        static void reportUnsafeAccessTo(Document* targetDocument);
 
     private:
         void resetIsolatedWorlds();