Prevent sandboxed iframe Document from sharing execution context with initial about:blank Document

This change fixes an issue where a sandboxed iframe can be created such that it contains a sandboxed Document with an opaque origin that still shares a script context with the iframe's initial un-sandboxed about:blank Document. The scenario is set up in the following manner: 1) Create a new iframe dynamically, and set its src to a same-domain page that we are going to sandbox. 2) Insert the iframe into a Document, and synchronously grab a reference to its initial about:blank Document. 3) Synchronously set iframe.sandbox = "allow-scripts" (this is still before the same-domain page has loaded in the frame). 4) The iframe’s navigation to the same-domain page occurs, asynchronously. FrameLoader::ShouldReuseDefaultView is called to determine the mode in which to load the new page. FrameLoader::ShouldReuseDefaultView fails to check the iframe’s sandbox flags (it only looks at the CSP ones), so the navigation proceeds without resetting the type system of the iframe. The result is that the newly loaded page shares the type system of the initial about:blank Document. 5) Code in the sandboxed iframe is now free to make changes to its type system that can affect any usage of the about:blank Document since they share the same type system. This is a sandbox escape in that if the same-domain page that the iframe is navigated to contains user-generated code, it could run outside the iframe. It can also result in crashes if we poke things in the right way, since an object that should be considered cross-origin can bleed into the top-level page, with the result that access checks which are never expected to fail can now fail. This change fixes the issue by making FrameLoader::ShouldReuseDefaultView() check the iframe's sandbox flags via FrameLoader::EffectiveSandboxFlags(), in addition to the existing check for CSP sandbox flags. Bug: 1017441 Change-Id: Ide1b13e16b0e0428a243ff47b6e17ae25ad0ff0d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1881315Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Nate Chapin <japhet@chromium.org> Commit-Queue: Dan Clark <daniec@microsoft.com> Cr-Commit-Position: refs/heads/master@{#710629}

Prevent sandboxed iframe Document from sharing execution context with initial about:blank Document
This change fixes an issue where a sandboxed iframe can be created such that it contains a sandboxed Document with an opaque origin that still shares a script context with the iframe's initial un-sandboxed about:blank Document. The scenario is set up in the following manner: 1) Create a new iframe dynamically, and set its src to a same-domain page that we are going to sandbox. 2) Insert the iframe into a Document, and synchronously grab a reference to its initial about:blank Document. 3) Synchronously set iframe.sandbox = "allow-scripts" (this is still before the same-domain page has loaded in the frame). 4) The iframe’s navigation to the same-domain page occurs, asynchronously. FrameLoader::ShouldReuseDefaultView is called to determine the mode in which to load the new page. FrameLoader::ShouldReuseDefaultView fails to check the iframe’s sandbox flags (it only looks at the CSP ones), so the navigation proceeds without resetting the type system of the iframe. The result is that the newly loaded page shares the type system of the initial about:blank Document. 5) Code in the sandboxed iframe is now free to make changes to its type system that can affect any usage of the about:blank Document since they share the same type system. This is a sandbox escape in that if the same-domain page that the iframe is navigated to contains user-generated code, it could run outside the iframe. It can also result in crashes if we poke things in the right way, since an object that should be considered cross-origin can bleed into the top-level page, with the result that access checks which are never expected to fail can now fail. This change fixes the issue by making FrameLoader::ShouldReuseDefaultView() check the iframe's sandbox flags via FrameLoader::EffectiveSandboxFlags(), in addition to the existing check for CSP sandbox flags. Bug: 1017441 Change-Id: Ide1b13e16b0e0428a243ff47b6e17ae25ad0ff0d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1881315Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Nate Chapin <japhet@chromium.org> Commit-Queue: Dan Clark <daniec@microsoft.com> Cr-Commit-Position: refs/heads/master@{#710629}
a6631386 · Daniel Clark · Commit Bot · bc8147b7 · a6631386 · a6631386
Commit a6631386 authored Oct 30, 2019 by Daniel Clark Committed by Commit Bot Oct 30, 2019
3 changed files
--- a/third_party/blink/renderer/core/loader/frame_loader.cc
+++ b/third_party/blink/renderer/core/loader/frame_loader.cc
@@ -1447,8 +1447,10 @@ bool FrameLoader::ShouldReuseDefaultView(
  // be considered when deciding whether to reuse it.
  // Spec:
  // https://html.spec.whatwg.org/C/#initialise-the-document-object
-  if (csp && (csp->GetSandboxMask() & WebSandboxFlags::kOrigin) !=
-                 WebSandboxFlags::kNone) {
+  if ((csp && (csp->GetSandboxMask() & WebSandboxFlags::kOrigin) !=
+                  WebSandboxFlags::kNone) ||
+      ((EffectiveSandboxFlags() & WebSandboxFlags::kOrigin) !=
+       WebSandboxFlags::kNone)) {
    return false;
  }


--- a/third_party/blink/web_tests/external/wpt/html/browsers/sandboxing/sandbox-new-execution-context-iframe.html
+++ b/third_party/blink/web_tests/external/wpt/html/browsers/sandboxing/sandbox-new-execution-context-iframe.html
+<body>
+  <script>
+    document.__proto__.changeFromSandboxedIframe = "change from sandboxed iframe";
+  </script>
+</body>
\ No newline at end of file
--- a/third_party/blink/web_tests/external/wpt/html/browsers/sandboxing/sandbox-new-execution-context.html
+++ b/third_party/blink/web_tests/external/wpt/html/browsers/sandboxing/sandbox-new-execution-context.html
+<!doctype html>
+<html>
+  <head>
+    <title>Reuse of iframe about:blank document execution context</title>
+    <link rel="author" title="Dan Clark" href="mailto:daniec@microsoft.com">
+    <link rel="help" href="http://www.w3.org/html/wg/drafts/html/master/browsers.html#sandboxing">
+    <script src="/resources/testharness.js"></script>
+    <script src="/resources/testharnessreport.js"></script>
+  </head>
+
+  <body>
+    <h1>Reuse of iframe about:blank document execution context in sandbox="allow-scripts" iframe</h1>
+    <script type="text/javascript">
+      var t = async_test("iframe with sandbox should load with new execution context")
+
+      let iframe = document.createElement("iframe");
+      iframe.src = './sandbox-new-execution-context-iframe.html';
+      document.body.appendChild(iframe);
+      iframe.sandbox = "allow-scripts";
+
+      let iframeAboutBlankDocument = iframe.contentDocument;
+      assert_equals(iframeAboutBlankDocument.URL, "about:blank");
+
+      iframe.onload = t.step_func_done(() => {
+        assert_equals(iframe.contentDocument, null,
+          "New document in sandboxed iframe should have opaque origin");
+
+        assert_equals(iframeAboutBlankDocument.__proto__.changeFromSandboxedIframe, undefined,
+          "Sandboxed iframe contents should not have been able to mess with type system of about:blank document");
+
+        let iframeAboutBlankContents = iframeAboutBlankDocument.querySelectorAll('body');
+        assert_equals(iframeAboutBlankContents[0].tagName, "BODY",
+          "about:blank document's contents should still be accessible");
+
+        t.done();
+      });
+    </script>
+    <div id="log"></div>
+  </body>
+</html>