Make error messages for cross-domain access OOPIF-friendly.

Source/bindings/core/v8/BindingSecurity.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 23 matching lines @@
 #include "bindings/core/v8/V8Binding.h"
 #include "core/dom/Document.h"
 #include "core/frame/LocalDOMWindow.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Settings.h"
 #include "core/html/HTMLFrameElementBase.h"
 #include "platform/weborigin/SecurityOrigin.h"
 
 namespace blink {
 
-static bool isDocumentAccessibleFromDOMWindow(Document* targetDocument, LocalDOMWindow* callingWindow)
+static bool isOriginAccessibleFromDOMWindow(SecurityOrigin* targetOrigin, LocalDOMWindow* callingWindow)
 {
-    if (!targetDocument)
-        return false;
+    return callingWindow && callingWindow->document()->securityOrigin()->canAccess(targetOrigin);
+}
 
-    if (!callingWindow)
-        return false;
-
-    if (callingWindow->document()->securityOrigin()->canAccess(targetDocument->securityOrigin()))
+static bool canAccessFrame(v8::Isolate* isolate, SecurityOrigin* targetFrameOrigin, DOMWindow* targetWindow, ExceptionState& exceptionState)
+{
+    LocalDOMWindow* callingWindow = callingDOMWindow(isolate);
+    if (isOriginAccessibleFromDOMWindow(targetFrameOrigin, callingWindow))
         return true;
 
+    if (targetWindow)
+        exceptionState.throwSecurityError(targetWindow->sanitizedCrossDomainAccessErrorMessage(callingWindow), targetWindow->crossDomainAccessErrorMessage(callingWindow));
     return false;
 }
 
-static bool canAccessDocument(v8::Isolate* isolate, Document* targetDocument, ExceptionState& exceptionState)
+static bool canAccessFrame(v8::Isolate* isolate, SecurityOrigin* targetFrameOrigin, DOMWindow* targetWindow, SecurityReportingOption reportingOption = ReportSecurityError)
 {
     LocalDOMWindow* callingWindow = callingDOMWindow(isolate);
-    if (isDocumentAccessibleFromDOMWindow(targetDocument, callingWindow))
+    if (isOriginAccessibleFromDOMWindow(targetFrameOrigin, callingWindow))
         return true;
 
-    if (targetDocument->domWindow())
-        exceptionState.throwSecurityError(targetDocument->domWindow()->sanitizedCrossDomainAccessErrorMessage(callingWindow), targetDocument->domWindow()->crossDomainAccessErrorMessage(callingWindow));
-    return false;
-}
-
-static bool canAccessDocument(v8::Isolate* isolate, Document* targetDocument, SecurityReportingOption reportingOption = ReportSecurityError)
-{
-    LocalDOMWindow* callingWindow = callingDOMWindow(isolate);
-    if (isDocumentAccessibleFromDOMWindow(targetDocument, callingWindow))
-        return true;
-
-    if (reportingOption == ReportSecurityError && targetDocument->domWindow())
-        callingWindow->printErrorMessage(targetDocument->domWindow()->crossDomainAccessErrorMessage(callingWindow));
-
+    if (reportingOption == ReportSecurityError && targetWindow)
+        callingWindow->printErrorMessage(targetWindow->crossDomainAccessErrorMessage(callingWindow));
     return false;
 }
 
 bool BindingSecurity::shouldAllowAccessToFrame(v8::Isolate* isolate, Frame* target, SecurityReportingOption reportingOption)
 {
-    if (!target || !target->isLocalFrame())
+    if (!target || !target->securityContext())
         return false;
-    return canAccessDocument(isolate, toLocalFrame(target)->document(), reportingOption);
+    return canAccessFrame(isolate, target->securityContext()->securityOrigin(), target->domWindow(), reportingOption);
 }
 
 bool BindingSecurity::shouldAllowAccessToFrame(v8::Isolate* isolate, Frame* target, ExceptionState& exceptionState)
 {
-    if (!target || !target->isLocalFrame())
+    if (!target || !target->securityContext())
         return false;
-    return canAccessDocument(isolate, toLocalFrame(target)->document(), exceptionState);
+    return canAccessFrame(isolate, target->securityContext()->securityOrigin(), target->domWindow(), exceptionState);
 }
 
 bool BindingSecurity::shouldAllowAccessToNode(v8::Isolate* isolate, Node* target, ExceptionState& exceptionState)
 {
-    return target && canAccessDocument(isolate, &target->document(), exceptionState);
+    return target && canAccessFrame(isolate, target->document().securityOrigin(), target->document().domWindow(), exceptionState);

[dcheng, 2015/04/23 17:39:57]

Would it make sense to have canAccessFrame() take a Frame*, and then we just
pass the target frame to it? That way, we don't have to pass the
SecurityOrigin/DOMWindow together like this.

[alexmos, 2015/04/23 17:45:28]

That was the first thing I tried, and unfortunately that didn't work, as
shouldAllowAccessToNode can be called with a target that has a Document with a
null frame() (This was exercised by
http/tests/inspector/template-content-inspect-crash.html).

[dcheng, 2015/04/23 18:01:24]

Do we actually return a true value in that case? Or would it return false
similar to lines 73-74 and 80-81?

[alexmos, 2015/04/23 21:25:23]

That particular test returns false - I think it has a DocumentFragment with a
unique origin and compares it to the test page's 127.0.0.1 origin.  But I'm not
sure that will always be false -- it seems DocumentFragments would pass their
creator Document's origin, which could be anything.  

That said, I'm also not sure under which circumstances shouldAllowAccessToNode
could get called for DocumentFragments.  Here's the call stack from the
inspector test, in case it's useful: 

#1  blink::BindingSecurity::shouldAllowAccessToNode (isolate=, target=,
exceptionState=...)
    at ../../third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp:87
#2  blink::InjectedScriptHost::nodeAsScriptValue (scriptState=, node=)
    at
../../third_party/WebKit/Source/bindings/core/v8/custom/V8InjectedScriptHostCustom.cpp:79
#3  blink::InspectableNode::get (this=, state=)
    at ../../third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:2160
#4  blink::V8InjectedScriptHost::inspectedObjectMethodCustom (info=...)
    at
../../third_party/WebKit/Source/bindings/core/v8/custom/V8InjectedScriptHostCustom.cpp:115
#5  blink::InjectedScriptHostV8Internal::inspectedObjectMethodCallback
(info=...)
    at gen/blink/bindings/core/v8/V8InjectedScriptHost.cpp:56
#6  v8::internal::FunctionCallbackArguments::Call (this=, 
   
f=<blink::InjectedScriptHostV8Internal::inspectedObjectMethodCallback(v8::FunctionCallbackInfo<v8::Value>
const&)>) at ../../v8/src/arguments.cc:33

 }
 
 }