Move securityCheck out of V8WrapperInstantiationScope

third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp

Index: third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp b/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp
index 516ef4dff5498d95b703de67ff7f23e7350fcaca..8b7184a48b8139452d7957bf6b703379dd695a22 100644
--- a/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp
@@ -271,4 +271,79 @@ void BindingSecurity::failedAccessCheckFor(v8::Isolate* isolate,
       targetWindow->crossDomainAccessErrorMessage(currentDOMWindow(isolate)));
 }
 
+bool BindingSecurity::canEnterCreationContext(
+    v8::Isolate* isolate,
+    v8::Local<v8::Context> currentContext,
+    v8::Local<v8::Context> creationContext,
+    const char* interfaceName) {
+  if (currentContext.IsEmpty() || creationContext.IsEmpty())

[Yuki, 2017/03/16 14:05:55]

Is that possible that currentContext.IsEmpty() nor creationContext.IsEmpty()? 
It's strange...

[adithyas, 2017/03/28 20:35:40]

This was just supposed to be currentContext.IsEmpty(), there's a RELEASE_ASSERT
for the creationContext earlier.

+    return false;
+
+  // If the context is different, we need to make sure that the current
+  // context has access to the creation context.
+  LocalFrame* frame = toLocalFrameIfNotDetached(creationContext);
+  if (!frame) {
+    // Sandbox detached frames - they can't create cross origin objects.
+    LocalDOMWindow* callingWindow = currentDOMWindow(isolate);
+    LocalDOMWindow* targetWindow = toLocalDOMWindow(creationContext);
+    ExceptionState exceptionState(isolate, ExceptionState::ConstructionContext,

[jbroman, 2017/03/15 19:58:41]

nit: This is the same as the ExceptionState below, and the one in
securityCheckForClassesWithoutAccessCheckCallbacks. Maybe just pass one from the
latter method in here?

[adithyas, 2017/03/28 20:35:40]

Done!

+                                  interfaceName);
+    if (shouldAllowAccessToDetachedWindow(callingWindow, targetWindow,
+                                          exceptionState)) {
+      return true;
+    }
+
+    CHECK_EQ(SecurityError, exceptionState.code());
+    return false;
+  }
+  const DOMWrapperWorld& currentWorld = DOMWrapperWorld::world(currentContext);
+  RELEASE_ASSERT(currentWorld.worldId() ==
+                 DOMWrapperWorld::world(creationContext).worldId());
+  ExceptionState exceptionState(isolate, ExceptionState::ConstructionContext,
+                                interfaceName);
+  if (currentWorld.isMainWorld() &&
+      !shouldAllowAccessToFrame(currentDOMWindow(isolate), frame,
+                                exceptionState)) {
+    CHECK_EQ(SecurityError, exceptionState.code());
+    return false;
+  }
+
+  return true;
+}
+
+void BindingSecurity::securityCheckForClassesWithAccessCheckCallbacks(
+    v8::Isolate* isolate,
+    v8::Local<v8::Context> currentContext,
+    v8::Local<v8::Context> creationContext,
+    const char* interfaceName,
+    v8::Local<v8::Value> crossContextException) {
+  // Classes with access check callbacks do allow some cross-origin accesses;
+  // the security checks are implemented in V8[[interfaceName]]::securityCheck.
+  if (!crossContextException.IsEmpty()) {
+    // Convert cross-context exception to security error
+    ExceptionState exceptionState(isolate, ExceptionState::ConstructionContext,

[Yuki, 2017/03/16 14:05:55]

In general, ExceptionState should be instantiated in the call sites to better
handle multiple possibilities of throwing an exception.  So, I'd recommend this
function to take an ExceptionState.  Plus, in this case, it looks strange that
you knew it's ExceptionState::ConstructionContext.

[adithyas, 2017/03/28 20:35:40]

I didn't create an exception state in the call site to remove a dependency to
ExceptionState (which I'm not planning on moving to platform/ initially) in
V8DOMWrapper. Also, this security check is only called when creating a wrapper,
so I think it's ok if it has knowledge of ExceptionState::ConstructionContext.

+                                  interfaceName);
+    LocalDOMWindow* callingWindow = currentDOMWindow(isolate);
+    LocalDOMWindow* targetWindow = toLocalDOMWindow(creationContext);
+    exceptionState.throwSecurityError(
+        targetWindow->sanitizedCrossDomainAccessErrorMessage(callingWindow),
+        targetWindow->crossDomainAccessErrorMessage(callingWindow));
+  };
+}
+
+void BindingSecurity::securityCheckForClassesWithoutAccessCheckCallbacks(
+    v8::Isolate* isolate,
+    v8::Local<v8::Context> currentContext,
+    v8::Local<v8::Context> creationContext,
+    const char* interfaceName,
+    v8::Local<v8::Value> crossContextException) {
+  if (canEnterCreationContext(isolate, currentContext, creationContext,
+                              interfaceName) &&
+      !crossContextException.IsEmpty()) {
+    ExceptionState exceptionState(isolate, ExceptionState::ConstructionContext,
+                                  interfaceName);
+    exceptionState.rethrowV8Exception(crossContextException);
+  }
+}
+
 }  // namespace blink