Fenced Frames: Fence named lookup of ancestors and related pages in S…

…hadowDOM We previously implemented fenced named lookup of descendants. This CL fences lookup of ancestors and related pages. The previous behavior in ShadowDOM was: * Named lookups of non-descendants in the same frame tree would fail due to a sandbox check, but this leaks information from the embedder into the fenced frame. * Named lookups of related pages would succeed if they were same-origin navigations, otherwise leak information. In MPArch, it works correctly by default. Bug: 1123606 Change-Id: I038a7f16146f0c11f079ed9a08b596eaa4074e9c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3557557 Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Dominic Farolino <dom@chromium.org> Commit-Queue: Garrett Tanzer <gtanzer@chromium.org> Cr-Commit-Position: refs/heads/main@{#987172}
chromium · Mar 30, 2022 · 01402e9 · 01402e9
1 parent 551335a
commit 01402e9
Show file tree

Hide file tree

Showing 4 changed files with 107 additions and 3 deletions.
diff --git a/third_party/blink/renderer/core/page/frame_tree.cc b/third_party/blink/renderer/core/page/frame_tree.cc
@@ -283,8 +283,10 @@ Frame* FrameTree::FindFrameForNavigationInternal(const AtomicString& name,
   if (!page)
     return nullptr;
 
-  for (Frame* frame = page->MainFrame(); frame;
-       frame = frame->Tree().TraverseNext()) {
+  for (Frame *top = &this_frame_->Tree().Top(FrameTreeBoundary::kFenced),
+             *frame = top;
+       frame;
+       frame = frame->Tree().TraverseNext(top, FrameTreeBoundary::kFenced)) {
     // Skip descendants of this frame that were searched above to avoid
     // showing duplicate console messages if a frame is found by name
     // but access is blocked.
@@ -295,6 +297,14 @@ Frame* FrameTree::FindFrameForNavigationInternal(const AtomicString& name,
     }
   }
 
+  // In fenced frames, only resolve target names using the above lookup methods
+  // (keywords, descendants, and the rest of the frame tree within the fence).
+  // TODO(crbug.com/1262022): Remove this early return when we get rid of
+  // ShadowDOM fenced frames, because it is unnecessary in MPArch.
+  if (this_frame_->IsInFencedFrameTree()) {
+    return nullptr;
+  }
+
   // Search the entire tree of each of the other pages in this namespace.
   for (const Page* other_page : page->RelatedPages()) {
     if (other_page == page || other_page->IsClosing())

diff --git a/third_party/blink/web_tests/wpt_internal/fenced_frame/navigate-ancestor-by-name.https.html b/third_party/blink/web_tests/wpt_internal/fenced_frame/navigate-ancestor-by-name.https.html
@@ -0,0 +1,67 @@
+<!DOCTYPE html>
+<title>Test named frame navigation of ancestors.</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/common/utils.js"></script>
+<script src="/common/dispatcher/dispatcher.js"></script>
+<script src="resources/utils.js"></script>
+
+<body>
+<script>
+promise_test(async () => {
+  // This test uses the following layout:
+  // A: Top-level frame
+  //   B: iframe
+  //     C: fencedframe
+  //       D: fencedframe
+  //       E: iframe
+  //
+  // The purpose is to test that name resolution of navigation targets ignores
+  // ancestors beyond fence boundaries.
+
+  // Create an iframe B.
+  const B = attachIFrameContext();
+  await B.execute(async () => {
+    window.name = "B";
+
+    // Create a fenced frame C inside of it.
+    window.C = attachFencedFrameContext();
+    await window.C.execute(async () => {
+      window.name = "C";
+
+      // Navigate the target "B" from inside the fenced frame.
+      // It should open in a new tab due to fenced name lookup.
+      window.open("resources/dummy.html", "B");
+    });
+  });
+
+  // Observe that it created a new window, and the frame B is still here.
+  await B.execute(async () => {
+    // Create a nested iframe and fenced frame.
+    await window.C.execute(async () => {
+      window.D = attachFencedFrameContext();
+      window.E = attachIFrameContext();
+
+      // Navigate the target "C" from inside the nested fenced frame.
+      // It should open in a new tab due to fenced name lookup.
+      await window.D.execute(async () => {
+        window.open("resources/dummy.html", "C");
+      });
+    });
+    // Observe that it created a new window, and the frame C is still here.
+    await window.C.execute(async () => {
+      // Now attempt to navigate the target "C" from inside the iframe.
+      // It should open in a new tab with a console error, because sandboxed
+      // iframes (inherited from the fenced frame) are not allowed to navigate
+      // their ancestors.
+      await window.E.execute(() => {
+        window.open("resources/dummy.html", "C");
+      });
+    });
+
+    // Observe that C is still here.
+    await window.C.execute(() => {});
+  });
+}, 'navigate named ancestors');
+</script>
+</body>
diff --git a/.../fenced_frame/navigate-by-name.https.html → ...me/navigate-descendant-by-name.https.html b/.../fenced_frame/navigate-by-name.https.html → ...me/navigate-descendant-by-name.https.html
@@ -1,5 +1,5 @@
 <!DOCTYPE html>
-<title>Test named frame navigation</title>
+<title>Test named frame navigation of descendants</title>
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <script src="resources/utils.js"></script>

diff --git a/..._party/blink/web_tests/wpt_internal/fenced_frame/navigate-related-page-by-name.https.html b/..._party/blink/web_tests/wpt_internal/fenced_frame/navigate-related-page-by-name.https.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<title>Test named frame navigation of related pages.</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/common/utils.js"></script>
+<script src="/common/dispatcher/dispatcher.js"></script>
+<script src="resources/utils.js"></script>
+
+<body>
+<script>
+promise_test(async () => {
+  // Create an auxiliary browsing context with a particular name.
+  const second_window = attachWindowContext({target: "target_name"});
+
+  // Create a fenced frame, and use the same target name inside of it.
+  const frame = attachFencedFrameContext();
+  await frame.execute(async () => {
+    window.open("resources/dummy.html", "target_name");
+  });
+
+  // Confirm that the top-level frame's related page (`second_window`)
+  // wasn't navigated by the fenced frame, i.e. that name resolution
+  // for related pages is fenced.
+  await second_window.execute(() => {});
+}, 'navigate related pages');
+</script>
+</body>