- Revision
- 124693
- Author
- [email protected]
- Date
- 2012-08-04 00:31:41 -0700 (Sat, 04 Aug 2012)
Log Message
[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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (124692 => 124693)
--- trunk/Source/WebCore/ChangeLog 2012-08-04 07:22:13 UTC (rev 124692)
+++ trunk/Source/WebCore/ChangeLog 2012-08-04 07:31:41 UTC (rev 124693)
@@ -1,3 +1,36 @@
+2012-08-04 Adam Barth <[email protected]>
+
+ [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 <[email protected]>
Disabling eval changes the timing of DidCreateScriptContext
Modified: trunk/Source/WebCore/bindings/generic/BindingSecurity.cpp (124692 => 124693)
--- trunk/Source/WebCore/bindings/generic/BindingSecurity.cpp 2012-08-04 07:22:13 UTC (rev 124692)
+++ trunk/Source/WebCore/bindings/generic/BindingSecurity.cpp 2012-08-04 07:31:41 UTC (rev 124693)
@@ -42,40 +42,28 @@
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 @@
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;
Modified: trunk/Source/WebCore/bindings/v8/BindingState.cpp (124692 => 124693)
--- trunk/Source/WebCore/bindings/v8/BindingState.cpp 2012-08-04 07:22:13 UTC (rev 124692)
+++ trunk/Source/WebCore/bindings/v8/BindingState.cpp 2012-08-04 07:31:41 UTC (rev 124693)
@@ -89,9 +89,9 @@
return V8Proxy::retrieveFrame(context);
}
-void immediatelyReportUnsafeAccessTo(BindingState*, Frame* target)
+void immediatelyReportUnsafeAccessTo(BindingState*, Document* targetDocument)
{
- V8Proxy::reportUnsafeAccessTo(target);
+ V8Proxy::reportUnsafeAccessTo(targetDocument);
}
}
Modified: trunk/Source/WebCore/bindings/v8/BindingState.h (124692 => 124693)
--- trunk/Source/WebCore/bindings/v8/BindingState.h 2012-08-04 07:22:13 UTC (rev 124692)
+++ trunk/Source/WebCore/bindings/v8/BindingState.h 2012-08-04 07:31:41 UTC (rev 124693)
@@ -34,6 +34,7 @@
namespace WebCore {
class DOMWindow;
+class Document;
class Frame;
class BindingState {
@@ -53,7 +54,7 @@
// are any subtle differences between the currentFrame and the lexicalGlobalObject.
Frame* currentFrame(BindingState*);
-void immediatelyReportUnsafeAccessTo(BindingState*, Frame*);
+void immediatelyReportUnsafeAccessTo(BindingState*, Document* targetDocument);
}
Modified: trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp (124692 => 124693)
--- trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp 2012-08-04 07:22:13 UTC (rev 124692)
+++ trunk/Source/WebCore/bindings/v8/V8DOMWindowShell.cpp 2012-08-04 07:31:41 UTC (rev 124693)
@@ -153,7 +153,7 @@
{
Frame* target = getTargetFrame(host, data);
if (target)
- V8Proxy::reportUnsafeAccessTo(target);
+ V8Proxy::reportUnsafeAccessTo(target->document());
}
PassRefPtr<V8DOMWindowShell> V8DOMWindowShell::create(Frame* frame)
Modified: trunk/Source/WebCore/bindings/v8/V8Proxy.cpp (124692 => 124693)
--- trunk/Source/WebCore/bindings/v8/V8Proxy.cpp 2012-08-04 07:22:13 UTC (rev 124692)
+++ trunk/Source/WebCore/bindings/v8/V8Proxy.cpp 2012-08-04 07:31:41 UTC (rev 124693)
@@ -124,13 +124,12 @@
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;
Modified: trunk/Source/WebCore/bindings/v8/V8Proxy.h (124692 => 124693)
--- trunk/Source/WebCore/bindings/v8/V8Proxy.h 2012-08-04 07:22:13 UTC (rev 124692)
+++ trunk/Source/WebCore/bindings/v8/V8Proxy.h 2012-08-04 07:31:41 UTC (rev 124693)
@@ -224,8 +224,7 @@
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();
