[shared storage] Disallow shared storage in opaque origin contexts

Why: This aligns with the handling for similar APIs such as localStorage. This should also fix a renderer crash, which is due to the fact that the browser's check for IsSecureFrame() would fail for opaque origin, while the renderer(IDL)'s check for SecureContext wouldn't fail for opaque origin. Bug: 1470628 Change-Id: I52a46ae11b2395b81b6c69236495b32b8e44fd2a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4790029 Reviewed-by: Cammie Smith Barnes <cammie@chromium.org> Commit-Queue: Yao Xiao <yaoxia@chromium.org> Cr-Commit-Position: refs/heads/main@{#1184938}
chromium · Aug 17, 2023 · 3685e92 · 3685e92
1 parent b24613a
commit 3685e92
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 3 deletions.
diff --git a/content/browser/shared_storage/shared_storage_document_service_impl.cc b/content/browser/shared_storage/shared_storage_document_service_impl.cc
@@ -31,6 +31,8 @@ namespace content {
 
 namespace {
 
+// Note that this function would also return false if the context origin is
+// opaque. This is stricter than the web platform's notion of "secure context".
 // TODO(yaoxia): This should be function in FrameTreeNode.
 bool IsSecureFrame(RenderFrameHost* frame) {
   while (frame) {
@@ -89,6 +91,13 @@ void SharedStorageDocumentServiceImpl::Bind(
   CHECK(!receiver_)
       << "Multiple attempts to bind the SharedStorageDocumentService receiver";
 
+  if (render_frame_host().GetLastCommittedOrigin().opaque()) {
+    mojo::ReportBadMessage(
+        "Attempted to request SharedStorageDocumentService from an opaque "
+        "origin context");
+    return;
+  }
+
   if (!IsSecureFrame(&render_frame_host())) {
     // This could indicate a compromised renderer, so let's terminate it.
     mojo::ReportBadMessage(

diff --git a/third_party/blink/renderer/modules/shared_storage/window_shared_storage.cc b/third_party/blink/renderer/modules/shared_storage/window_shared_storage.cc
@@ -54,7 +54,15 @@ const char WindowSharedStorageImpl::kSupplementName[] =
 
 }  // namespace
 
-SharedStorage* WindowSharedStorage::sharedStorage(LocalDOMWindow& window) {
+SharedStorage* WindowSharedStorage::sharedStorage(
+    LocalDOMWindow& window,
+    ExceptionState& exception_state) {
+  if (window.GetSecurityOrigin()->IsOpaque()) {
+    exception_state.ThrowSecurityError(
+        "sharedStorage is not allowed in an opaque origin context");
+    return nullptr;
+  }
+
   return WindowSharedStorageImpl::From(window).GetOrCreate(window);
 }
 

diff --git a/third_party/blink/renderer/modules/shared_storage/window_shared_storage.h b/third_party/blink/renderer/modules/shared_storage/window_shared_storage.h
@@ -9,6 +9,7 @@
 
 namespace blink {
 
+class ExceptionState;
 class SharedStorage;
 class LocalDOMWindow;
 
@@ -17,7 +18,7 @@ class WindowSharedStorage {
   STATIC_ONLY(WindowSharedStorage);
 
  public:
-  static SharedStorage* sharedStorage(LocalDOMWindow&);
+  static SharedStorage* sharedStorage(LocalDOMWindow&, ExceptionState&);
 };
 
 }  // namespace blink

diff --git a/third_party/blink/renderer/modules/shared_storage/window_shared_storage.idl b/third_party/blink/renderer/modules/shared_storage/window_shared_storage.idl
@@ -4,6 +4,7 @@
 ] partial interface Window {
   [
     MeasureAs=SharedStorageAPI_SharedStorage_DOMReference,
-    SecureContext
+    SecureContext,
+    RaisesException
   ] readonly attribute SharedStorage sharedStorage;
 };
diff --git a/...ty/blink/web_tests/external/wpt/shared-storage/resources/verify-shared-storage.https.html b/...ty/blink/web_tests/external/wpt/shared-storage/resources/verify-shared-storage.https.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script>
+  try {
+    window.sharedStorage;
+    window.parent.postMessage({ accessSharedStorageResult: 'success'}, "*");
+  } catch (error) {
+    window.parent.postMessage({ accessSharedStorageResult: 'failure'}, "*");
+  }
+</script>
+</body>
+</html>
diff --git a/...tests/external/wpt/shared-storage/shared-storage-in-sandboxed-iframe.tentative.https.html b/...tests/external/wpt/shared-storage/shared-storage-in-sandboxed-iframe.tentative.https.html
@@ -0,0 +1,41 @@
+<!doctype html>
+<body>
+  <script src=/resources/testharness.js></script>
+  <script src=/resources/testharnessreport.js></script>
+  <script src=/browsing-topics/resources/header-util.sub.js></script>
+  <script>
+    function test_shared_storage_in_sandboxed_iframe(test, sandbox_flags, expect_success) {
+      let frame = document.createElement('iframe');
+      frame.sandbox = sandbox_flags;
+      frame.src = '/shared-storage/resources/verify-shared-storage.https.html';
+
+      window.addEventListener('message', test.step_func(function handler(evt) {
+        if (evt.source === frame.contentWindow) {
+          if (expect_success) {
+            assert_equals(evt.data.accessSharedStorageResult, 'success');
+          } else {
+            assert_equals(evt.data.accessSharedStorageResult, 'failure');
+          }
+
+          document.body.removeChild(frame);
+          window.removeEventListener('message', handler);
+          test.done();
+        }
+      }));
+
+      document.body.appendChild(frame);
+    }
+
+    async_test(t => {
+      test_shared_storage_in_sandboxed_iframe(t,
+        /*sandbox_flags=*/'allow-scripts allow-same-origin',
+        /*expect_success=*/true);
+    }, 'test shared storage in sandboxed iframe with "allow-same-origin"');
+
+    async_test(t => {
+      test_shared_storage_in_sandboxed_iframe(t,
+        /*sandbox_flags=*/'allow-scripts',
+        /*expect_success=*/false);
+    }, 'test shared storage in sandboxed iframe without "allow-same-origin"');
+  </script>
+</body>