Rework security checks to be based on Window rather than Frame.

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 15 matching lines @@
  * 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/V8BindingForCore.h"
 #include "bindings/core/v8/V8Location.h"
+#include "bindings/core/v8/V8Window.h"
 #include "core/dom/Document.h"
+#include "core/frame/DOMWindow.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/bindings/WrapperCreationSecurityCheck.h"
 #include "platform/weborigin/SecurityOrigin.h"
 
 namespace blink {
 
 namespace {
 
-bool CanAccessFrameInternal(const LocalDOMWindow* accessing_window,
-                            const SecurityOrigin* target_frame_origin,
-                            const DOMWindow* target_window) {
+bool CanAccessWindowInternal(const LocalDOMWindow* accessing_window,
+                             const DOMWindow* target_window) {
   SECURITY_CHECK(!(target_window && target_window->GetFrame()) ||
                  target_window == target_window->GetFrame()->DomWindow());
 
   // It's important to check that target_window is a LocalDOMWindow: it's
   // possible for a remote frame and local frame to have the same security
   // origin, depending on the model being used to allocate Frames between
   // processes. See https://crbug.com/601629.
   if (!(accessing_window && target_window && target_window->IsLocalDOMWindow()))
     return false;
 
   const SecurityOrigin* accessing_origin =
       accessing_window->document()->GetSecurityOrigin();
-  if (!accessing_origin->CanAccess(target_frame_origin))
+  const LocalDOMWindow* local_target_window = ToLocalDOMWindow(target_window);
+  if (!accessing_origin->CanAccess(
+          local_target_window->document()->GetSecurityOrigin())) {
     return false;
+  }
 
   // Notify the loader's client if the initial document has been accessed.
-  LocalFrame* target_frame = ToLocalDOMWindow(target_window)->GetFrame();
+  LocalFrame* target_frame = local_target_window->GetFrame();
   if (target_frame &&
       target_frame->Loader().StateMachine()->IsDisplayingInitialEmptyDocument())
     target_frame->Loader().DidAccessInitialDocument();
 
   return true;
 }
 
-bool CanAccessFrame(const LocalDOMWindow* accessing_window,
-                    const SecurityOrigin* target_frame_origin,
-                    const DOMWindow* target_window,
-                    ExceptionState& exception_state) {
-  if (CanAccessFrameInternal(accessing_window, target_frame_origin,
-                             target_window))
+bool CanAccessWindow(const LocalDOMWindow* accessing_window,
+                     const DOMWindow* target_window,
+                     ExceptionState& exception_state) {
+  if (CanAccessWindowInternal(accessing_window, target_window))
     return true;
 
   if (target_window)
     exception_state.ThrowSecurityError(
         target_window->SanitizedCrossDomainAccessErrorMessage(accessing_window),
         target_window->CrossDomainAccessErrorMessage(accessing_window));
   return false;
 }
 
-bool CanAccessFrame(const LocalDOMWindow* accessing_window,
-                    SecurityOrigin* target_frame_origin,
-                    const DOMWindow* target_window,
-                    BindingSecurity::ErrorReportOption reporting_option) {
-  if (CanAccessFrameInternal(accessing_window, target_frame_origin,
-                             target_window))
+bool CanAccessWindow(const LocalDOMWindow* accessing_window,
+                     const DOMWindow* target_window,
+                     BindingSecurity::ErrorReportOption reporting_option) {
+  if (CanAccessWindowInternal(accessing_window, target_window))
     return true;
 
   if (accessing_window && target_window &&
       reporting_option == BindingSecurity::ErrorReportOption::kReport)
     accessing_window->PrintErrorMessage(
         target_window->CrossDomainAccessErrorMessage(accessing_window));
   return false;
 }
 
+DOMWindow* FindWindow(v8::Isolate* isolate,
+                      const WrapperTypeInfo* type,
+                      v8::Local<v8::Object> holder) {
+  if (V8Window::wrapperTypeInfo.Equals(type))
+    return V8Window::toImpl(holder);
+
+  if (V8Location::wrapperTypeInfo.Equals(type))
+    return V8Location::toImpl(holder)->DomWindow();
+
+  // This function can handle only those types listed above.
+  NOTREACHED();
+  return nullptr;
+}
+
 }  // namespace
 
 bool BindingSecurity::ShouldAllowAccessTo(
     const LocalDOMWindow* accessing_window,
     const DOMWindow* target,
     ExceptionState& exception_state) {
   DCHECK(target);
-  const Frame* frame = target->GetFrame();
-  if (!frame || !frame->GetSecurityContext())
+
+  // TODO(https://crbug.com/723057): This is intended to match the legacy
+  // behavior of when access checks revolved around Frame pointers rather than
+  // DOMWindow pointers. This prevents web-visible behavior changes, since the
+  // previous implementation had to follow the back pointer to the Frame, and
+  // would have to early return when it was null.
+  if (!target->GetFrame())
     return false;
-  return CanAccessFrame(accessing_window,
-                        frame->GetSecurityContext()->GetSecurityOrigin(),
-                        target, exception_state);
+
+  return CanAccessWindow(accessing_window, target, exception_state);
 }
 
 bool BindingSecurity::ShouldAllowAccessTo(
     const LocalDOMWindow* accessing_window,
     const DOMWindow* target,
     ErrorReportOption reporting_option) {
   DCHECK(target);
-  const Frame* frame = target->GetFrame();
-  if (!frame || !frame->GetSecurityContext())
+
+  // TODO(https://crbug.com/723057): This is intended to match the legacy
+  // behavior of when access checks revolved around Frame pointers rather than
+  // DOMWindow pointers. This prevents web-visible behavior changes, since the
+  // previous implementation had to follow the back pointer to the Frame, and
+  // would have to early return when it was null.
+  if (!target->GetFrame())
     return false;
-  return CanAccessFrame(accessing_window,
-                        frame->GetSecurityContext()->GetSecurityOrigin(),
-                        target, reporting_option);
+
+  return CanAccessWindow(accessing_window, target, reporting_option);
 }
 
 bool BindingSecurity::ShouldAllowAccessTo(
-    const LocalDOMWindow* accessing_window,
-    const EventTarget* target,
-    ExceptionState& exception_state) {
-  DCHECK(target);
-  const DOMWindow* window = target->ToDOMWindow();
-  if (!window) {
-    // We only need to check the access to Window objects which are
-    // cross-origin accessible.  If it's not a Window, the object's
-    // origin must always be the same origin (or it already leaked).
-    return true;
-  }
-  const Frame* frame = window->GetFrame();
-  if (!frame || !frame->GetSecurityContext())
-    return false;
-  return CanAccessFrame(accessing_window,
-                        frame->GetSecurityContext()->GetSecurityOrigin(),
-                        window, exception_state);
-}
-
-bool BindingSecurity::ShouldAllowAccessTo(
     const LocalDOMWindow* accessing_window,
     const Location* target,
     ExceptionState& exception_state) {
   DCHECK(target);
-  const Frame* frame = target->GetFrame();
-  if (!frame || !frame->GetSecurityContext())
+
+  // TODO(https://crbug.com/723057): This is intended to match the legacy
+  // behavior of when access checks revolved around Frame pointers rather than
+  // DOMWindow pointers. This prevents web-visible behavior changes, since the
+  // previous implementation had to follow the back pointer to the Frame, and
+  // would have to early return when it was null.
+  if (!target->DomWindow()->GetFrame())
     return false;
-  return CanAccessFrame(accessing_window,
-                        frame->GetSecurityContext()->GetSecurityOrigin(),
-                        frame->DomWindow(), exception_state);
+
+  return CanAccessWindow(accessing_window, target->DomWindow(),
+                         exception_state);
 }
 
 bool BindingSecurity::ShouldAllowAccessTo(
     const LocalDOMWindow* accessing_window,
     const Location* target,
     ErrorReportOption reporting_option) {
   DCHECK(target);
-  const Frame* frame = target->GetFrame();
-  if (!frame || !frame->GetSecurityContext())
+
+  // TODO(https://crbug.com/723057): This is intended to match the legacy
+  // behavior of when access checks revolved around Frame pointers rather than
+  // DOMWindow pointers. This prevents web-visible behavior changes, since the
+  // previous implementation had to follow the back pointer to the Frame, and
+  // would have to early return when it was null.
+  if (!target->DomWindow()->GetFrame())
     return false;
-  return CanAccessFrame(accessing_window,
-                        frame->GetSecurityContext()->GetSecurityOrigin(),
-                        frame->DomWindow(), reporting_option);
+
+  return CanAccessWindow(accessing_window, target->DomWindow(),
+                         reporting_option);
 }
 
 bool BindingSecurity::ShouldAllowAccessTo(
     const LocalDOMWindow* accessing_window,
     const Node* target,
     ExceptionState& exception_state) {
   if (!target)
     return false;
-  return CanAccessFrame(accessing_window,
-                        target->GetDocument().GetSecurityOrigin(),
-                        target->GetDocument().domWindow(), exception_state);
+  return CanAccessWindow(accessing_window, target->GetDocument().domWindow(),
+                         exception_state);
 }
 
 bool BindingSecurity::ShouldAllowAccessTo(
     const LocalDOMWindow* accessing_window,
     const Node* target,
     ErrorReportOption reporting_option) {
   if (!target)
     return false;
-  return CanAccessFrame(accessing_window,
-                        target->GetDocument().GetSecurityOrigin(),
-                        target->GetDocument().domWindow(), reporting_option);
+  return CanAccessWindow(accessing_window, target->GetDocument().domWindow(),
+                         reporting_option);
 }
 
 bool BindingSecurity::ShouldAllowAccessToFrame(
     const LocalDOMWindow* accessing_window,
     const Frame* target,
     ExceptionState& exception_state) {
   if (!target || !target->GetSecurityContext())
     return false;
-  return CanAccessFrame(accessing_window,
-                        target->GetSecurityContext()->GetSecurityOrigin(),
-                        target->DomWindow(), exception_state);
+  return CanAccessWindow(accessing_window, target->DomWindow(),
+                         exception_state);
 }
 
 bool BindingSecurity::ShouldAllowAccessToFrame(
     const LocalDOMWindow* accessing_window,
     const Frame* target,
     ErrorReportOption reporting_option) {
   if (!target || !target->GetSecurityContext())
     return false;
-  return CanAccessFrame(accessing_window,
-                        target->GetSecurityContext()->GetSecurityOrigin(),
-                        target->DomWindow(), reporting_option);
-}
-
-bool BindingSecurity::ShouldAllowAccessToDetachedWindow(
-    const LocalDOMWindow* accessing_window,
-    const DOMWindow* target,
-    ExceptionState& exception_state) {
-  CHECK(target && !target->GetFrame())
-      << "This version of shouldAllowAccessToFrame() must be used only for "
-      << "detached windows.";
-  if (!target->IsLocalDOMWindow())
-    return false;
-  Document* document = ToLocalDOMWindow(target)->document();
-  if (!document)
-    return false;
-  return CanAccessFrame(accessing_window, document->GetSecurityOrigin(), target,
-                        exception_state);
+  return CanAccessWindow(accessing_window, target->DomWindow(),
+                         reporting_option);
 }
 
 bool BindingSecurity::ShouldAllowNamedAccessTo(
     const DOMWindow* accessing_window,
     const DOMWindow* target_window) {
   const Frame* accessing_frame = accessing_window->GetFrame();
   DCHECK(accessing_frame);
   DCHECK(accessing_frame->GetSecurityContext());
   const SecurityOrigin* accessing_origin =
       accessing_frame->GetSecurityContext()->GetSecurityOrigin();
@@ skipping 24 matching lines @@
   // 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;
 
   v8::Isolate* isolate = creation_context->GetIsolate();
   LocalFrame* frame = ToLocalFrameIfNotDetached(creation_context);
   ExceptionState exception_state(isolate, ExceptionState::kConstructionContext,
                                  type->interface_name);
+  // TODO(dcheng): Why doesn't this code just use DOMWindows throughout? Can't
+  // we just always use ToLocalDOMWindow(creation_context)?
   if (!frame) {
     // Sandbox detached frames - they can't create cross origin objects.
     LocalDOMWindow* calling_window = CurrentDOMWindow(isolate);
     LocalDOMWindow* target_window = ToLocalDOMWindow(creation_context);
 
-    return ShouldAllowAccessToDetachedWindow(calling_window, target_window,
-                                             exception_state);
+    // TODO(https://crbug.com/723057): This is tricky: this intentionally uses
+    // the internal CanAccessWindow() helper rather than ShouldAllowAccessTo().
+    // ShouldAllowAccessTo() unconditionally denies access if the DOMWindow is
+    // not attached to a Frame, but this code is intended for handling the
+    // detached DOMWindow case.
+    return CanAccessWindow(calling_window, target_window, exception_state);
   }
   const DOMWrapperWorld& current_world =
       DOMWrapperWorld::World(isolate->GetCurrentContext());
   CHECK_EQ(current_world.GetWorldId(),
            DOMWrapperWorld::World(creation_context).GetWorldId());
 
   return !current_world.IsMainWorld() ||
          ShouldAllowAccessToFrame(CurrentDOMWindow(isolate), frame,
                                   exception_state);
 }
@@ skipping 19 matching lines @@
 }
 
 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)
+                                           const WrapperTypeInfo* type,
+                                           v8::Local<v8::Object> holder) {
+  DOMWindow* target = FindWindow(isolate, type, holder);
+  // Failing to find a target means something is wrong. Failing to throw an
+  // exception could be a security issue, so just crash.
+  CHECK(target);
+
+  // TODO(https://crbug.com/723057): This is intended to match the legacy
+  // behavior of when access checks revolved around Frame pointers rather than
+  // DOMWindow pointers. This prevents web-visible behavior changes, since the
+  // previous implementation had to follow the back pointer to the Frame, and
+  // would have to early return when it was null.
+  if (!target->GetFrame())
     return;
 
-  DOMWindow* target_window = target->DomWindow();
-
   // TODO(dcheng): Add ContextType, interface name, and property name as
   // arguments, so the generated exception can be more descriptive.
   ExceptionState exception_state(isolate, ExceptionState::kUnknownContext,
                                  nullptr, nullptr);
   exception_state.ThrowSecurityError(
-      target_window->SanitizedCrossDomainAccessErrorMessage(
-          CurrentDOMWindow(isolate)),
-      target_window->CrossDomainAccessErrorMessage(CurrentDOMWindow(isolate)));
+      target->SanitizedCrossDomainAccessErrorMessage(CurrentDOMWindow(isolate)),
+      target->CrossDomainAccessErrorMessage(CurrentDOMWindow(isolate)));
 }
 
 }  // namespace blink