Move securityCheck out of V8WrapperInstantiationScope

third_party/WebKit/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 14 matching lines @@
  * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "bindings/core/v8/BindingSecurity.h"
 
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/V8Binding.h"
+#include "bindings/core/v8/V8Location.h"
+#include "bindings/core/v8/WrapperCreationSecurityCheck.h"
 #include "core/dom/Document.h"
 #include "core/frame/LocalDOMWindow.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Location.h"
 #include "core/frame/Settings.h"
 #include "core/html/HTMLFrameElementBase.h"
 #include "core/workers/MainThreadWorkletGlobalScope.h"
 #include "platform/weborigin/SecurityOrigin.h"
 
 namespace blink {
@@ skipping 200 matching lines @@
     return false;
 
   // Note that there is no need to call back
   // FrameLoader::didAccessInitialDocument() because |targetWindow| must be
   // a child window inside iframe or frame and it doesn't have a URL bar,
   // so there is no need to worry about URL spoofing.
 
   return true;
 }
 
+bool BindingSecurity::shouldAllowAccessToCreationContext(
+    v8::Isolate* isolate,

[haraken, 2017/04/09 15:28:59]

You don't need to pass in an Isolate*, since you can get it by
creationContext->GetIsolate().

[adithyas, 2017/04/10 18:02:00]

Fixed, here and everywhere else.

+    v8::Local<v8::Context> creationContext,
+    const WrapperTypeInfo* type) {
+  // According to
+  // https://html.spec.whatwg.org/multipage/browsers.html#security-location,
+  // cross-origin script access to a few properties of Location is allowed.
+  // Location already implements the necessary security checks.
+  if (type->equals(&V8Location::wrapperTypeInfo))
+    return true;
+
+  LocalFrame* frame = toLocalFrameIfNotDetached(creationContext);
+  ExceptionState exceptionState(isolate, ExceptionState::ConstructionContext,
+                                type->interfaceName);
+  if (!frame) {
+    // Sandbox detached frames - they can't create cross origin objects.
+    LocalDOMWindow* callingWindow = currentDOMWindow(isolate);
+    LocalDOMWindow* targetWindow = toLocalDOMWindow(creationContext);
+
+    return shouldAllowAccessToDetachedWindow(callingWindow, targetWindow,
+                                             exceptionState);
+  }
+  const DOMWrapperWorld& currentWorld =
+      DOMWrapperWorld::world(isolate->GetCurrentContext());
+  CHECK_EQ(currentWorld.worldId(),
+           DOMWrapperWorld::world(creationContext).worldId());
+
+  return !currentWorld.isMainWorld() ||

[haraken, 2017/04/09 15:29:00]

I'd like to understand what the isMainWorld check is for. Would you mind
investigating a git blame history a bit?

[adithyas, 2017/04/10 18:02:00]

This is the CL that added the check in:
https://codereview.chromium.org/1323453005#msg15

jochen@'s comment explains it: "the security token of the isolated world is
chrome-extension://abcd..., so the security check against the Frame's security
origin will always fail.

so for isolated worlds, all we check is that the access is within the same
isolated world."

+         shouldAllowAccessToFrame(currentDOMWindow(isolate), frame,
+                                  exceptionState);
+}
+
+void BindingSecurity::rethrowCrossContextException(
+    v8::Isolate* isolate,

[haraken, 2017/04/09 15:29:00]

Ditto. Remove the Isolate* parameter.

+    v8::Local<v8::Context> creationContext,
+    const WrapperTypeInfo* type,
+    v8::Local<v8::Value> crossContextException) {
+  DCHECK(!crossContextException.IsEmpty());
+  ExceptionState exceptionState(isolate, ExceptionState::ConstructionContext,
+                                type->interfaceName);

[haraken, 2017/04/09 15:29:00]

It looks redundant to create an ExceptionState. I think you can just make
BindingSecurity::rethrowCrossContextException a V8 exception. You can address it
in a follow-up CL.

[Yuki, 2017/04/10 04:13:30]

Yes, it's somewhat redundant, but ExceptionState supports creating an exception
message (in a somewhat unified format).  I think that we want to take its
advantage.  In general, it's not on a performance-sensitive path if it's
throwing an exception.

+  if (type->equals(&V8Location::wrapperTypeInfo)) {
+    // Convert cross-context exception to security error
+    LocalDOMWindow* callingWindow = currentDOMWindow(isolate);
+    LocalDOMWindow* targetWindow = toLocalDOMWindow(creationContext);
+    exceptionState.throwSecurityError(
+        targetWindow->sanitizedCrossDomainAccessErrorMessage(callingWindow),
+        targetWindow->crossDomainAccessErrorMessage(callingWindow));
+    return;
+  }
+  exceptionState.rethrowV8Exception(crossContextException);
+}
+
+void BindingSecurity::initWrapperCreationSecurityCheck() {
+  WrapperCreationSecurityCheck::setSecurityCheckFunction(
+      shouldAllowAccessToCreationContext);
+  WrapperCreationSecurityCheck::setRethrowExceptionFunction(
+      rethrowCrossContextException);
+}
+
 void BindingSecurity::failedAccessCheckFor(v8::Isolate* isolate,
                                            const Frame* target) {
   // TODO(dcheng): See if this null check can be removed or hoisted to a
   // different location.
   if (!target)
     return;
 
   DOMWindow* targetWindow = target->domWindow();
 
   // TODO(dcheng): Add ContextType, interface name, and property name as
   // arguments, so the generated exception can be more descriptive.
   ExceptionState exceptionState(isolate, ExceptionState::UnknownContext,
                                 nullptr, nullptr);
   exceptionState.throwSecurityError(
       targetWindow->sanitizedCrossDomainAccessErrorMessage(
           currentDOMWindow(isolate)),
       targetWindow->crossDomainAccessErrorMessage(currentDOMWindow(isolate)));
 }
 
 }  // namespace blink