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 253 matching lines @@
   // 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)));
 }
 
+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