From d2bc6331aa0de58aa458bf92c9b13f81255b43b3 Mon Sep 17 00:00:00 2001
From: Glenn Hartmann <[email protected]>
Date: Wed, 23 Sep 2026 07:39:34 -0700
Subject: [PATCH] Preserve the Web Share Target initiator origin through launch

crrev.com/c/8415706 attributed Web Share Target navigations to the
sharing app, so that an in-scope action that redirects cross-site no
longer carries the target site's SameSite=Strict/Lax cookies. Two of
the three initiator origins it set are overwritten further down the
stack, so the fix only took effect on the WebAPK/TWA POST path.

WebAppLaunchProcess::Run() assigns params_->initiator_origin over
whatever NavigateParamsForShareTarget() computed. Share launches carry
no initiator, so the app attribution is reset to std::nullopt and the
navigation is again treated as browser-initiated, leaving the original
SameSite/CSRF bypass in place on desktop and ChromeOS. Only overwrite
the initiator origin when the launch actually has one. Every other
launch builds its NavigateParams fresh with std::nullopt, so this is a
no-op for them.

CustomTabActivityNavigationController::navigate() replaces the
initiator with an opaque origin because the sender of an intent can't
be trusted. That is the right default, but it also discards the
verified origin TwaSharingController sets for GET share targets, which
drops Strict cookies on the initial navigation and breaks signed-in
share targets -- the behavior change the earlier CL set out to avoid.
This direction fails closed, so it is not a cookie leak. Only fall back
to an opaque origin when the caller has not already attributed the
navigation; TwaSharingController is the only caller that sets one.

Each new test was confirmed to fail with its fix reverted.

TAG=agy
CONV=7915d2d9-4027-4cad-827e-e9232a49551b

Bug: 40061291
Fixed: 562714408
Change-Id: I302ab55125dbe47bf6bc4728d22a3e3d6a6a6964
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8429148
Commit-Queue: Glenn Hartmann <[email protected]>
Reviewed-by: Yaron Friedman <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1703409}
---
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityNavigationController.java b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityNavigationController.java
index ce4570cd..0102b2fb 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityNavigationController.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityNavigationController.java
@@ -252,8 +252,13 @@
                         assertNonNull(mIntentDataProvider.getIntent()), transition));
 
         // The sender of an intent can't be trusted, so we navigate from an opaque Origin to
-        // avoid sending same-site cookies.
-        params.setInitiatorOrigin(Origin.createOpaqueOrigin());
+        // avoid sending same-site cookies. Callers that have verified who really initiated the
+        // navigation (e.g. TWA share targets, whose action URL is verified to be within the
+        // app's scope) set the initiator themselves; don't clobber that. See
+        // crbug.com/40061291.
+        if (params.getInitiatorOrigin() == null) {
+            params.setInitiatorOrigin(Origin.createOpaqueOrigin());
+        }
 
         // Notifies PreloadingImpl that a navigation to CCT is happening. This is used to calculate
         // the recall of CCT prefetch's attempt. Please see
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityUrlLoadingTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityUrlLoadingTest.java
index 1fbada7..7ee2d02 100644
--- a/chrome/android/junit/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityUrlLoadingTest.java
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityUrlLoadingTest.java
@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.customtabs.content;
 
+import static org.junit.Assert.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.Mockito.clearInvocations;
@@ -181,6 +182,42 @@
         verify(env.tabFromFactory).loadUrl(argThat(params -> OTHER_URL.equals(params.getUrl())));
     }
 
+    /**
+     * The sender of an intent can't be trusted, so an unattributed navigation must be made from an
+     * opaque origin rather than inheriting same-site cookies.
+     */
+    @Test
+    public void navigatesFromOpaqueOrigin_IfNoInitiatorOriginSpecified() {
+        Origin opaqueOrigin = mock(Origin.class);
+        Origin.setOpaqueOriginFactoryForTesting(() -> opaqueOrigin);
+        mTabController.setUpInitialTab(null);
+        mTabController.finishNativeInitialization();
+
+        LoadUrlParams params = new LoadUrlParams(OTHER_URL);
+        mNavigationController.navigate(params, new Intent());
+
+        assertEquals(opaqueOrigin, params.getInitiatorOrigin());
+    }
+
+    /**
+     * Regression test for crbug.com/40061291: callers that have verified the real initiator (e.g.
+     * TWA share targets, whose action is verified to be in the app's scope) set it themselves, and
+     * it must survive so that SameSite cookies are evaluated against it.
+     */
+    @Test
+    public void keepsInitiatorOrigin_IfSpecifiedByCaller() {
+        Origin.setOpaqueOriginFactoryForTesting(() -> mock(Origin.class));
+        Origin appOrigin = mock(Origin.class);
+        mTabController.setUpInitialTab(null);
+        mTabController.finishNativeInitialization();
+
+        LoadUrlParams params = new LoadUrlParams(OTHER_URL);
+        params.setInitiatorOrigin(appOrigin);
+        mNavigationController.navigate(params, new Intent());
+
+        assertEquals(appOrigin, params.getInitiatorOrigin());
+    }
+
     private CustomTabIntentDataProvider createDataProviderForNewIntent() {
         CustomTabIntentDataProvider dataProvider = mock(CustomTabIntentDataProvider.class);
         when(dataProvider.getUrlToLoad()).thenReturn(OTHER_URL);
diff --git a/chrome/browser/ui/web_applications/web_app_launch_process.cc b/chrome/browser/ui/web_applications/web_app_launch_process.cc
index e6f2ac1..71834f8 100644
--- a/chrome/browser/ui/web_applications/web_app_launch_process.cc
+++ b/chrome/browser/ui/web_applications/web_app_launch_process.cc
@@ -272,8 +272,8 @@
   // In the case of prevent-close, we do not navigate but instead focus the
   // existing window.
   bool app_wants_focus_existing_without_navigation =
-     GetLaunchHandler().NeverNavigateExistingClients() ||
-     registrar_->IsPreventCloseEnabled(params_->app_id);
+      GetLaunchHandler().NeverNavigateExistingClients() ||
+      registrar_->IsPreventCloseEnabled(params_->app_id);
   if (!open_in_new_window && app_wants_focus_existing_without_navigation) {
     auto* tab_helper = WebAppTabHelper::FromWebContents(existing_tab);
     if (tab_helper->pending_launch_app_id() == params_->app_id) {
@@ -313,7 +313,15 @@
           : NavigateParams(browser, url_to_navigate,
                            ui::PAGE_TRANSITION_AUTO_BOOKMARK);
   nav_params.disposition = navigation_disposition;
-  nav_params.initiator_origin = params_->initiator_origin;
+  // Only overwrite the initiator origin when the launch actually carries one.
+  // `NavigateParamsForShareTarget()` attributes share target navigations to the
+  // app itself, and share launches have no `initiator_origin`; assigning it
+  // unconditionally would reset that back to std::nullopt, which makes the
+  // navigation look like a user-typed URL to the network stack. See
+  // crbug.com/40061291.
+  if (params_->initiator_origin.has_value()) {
+    nav_params.initiator_origin = params_->initiator_origin;
+  }
   if (params_->initiator_origin.has_value() ||
       !params_->referrer_url.is_empty()) {
     nav_params.referrer = content::Referrer::SanitizeForRequest(
diff --git a/chrome/browser/ui/web_applications/web_share_target_browsertest.cc b/chrome/browser/ui/web_applications/web_share_target_browsertest.cc
index 4309787..24ea08c 100644
--- a/chrome/browser/ui/web_applications/web_share_target_browsertest.cc
+++ b/chrome/browser/ui/web_applications/web_share_target_browsertest.cc
@@ -42,12 +42,14 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_navigation_observer.h"
 #include "net/base/filename_util.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "services/network/public/cpp/resource_request_body.h"
 #include "storage/browser/file_system/file_system_context.h"
 #include "ui/display/types/display_constants.h"
 #include "url/gurl.h"
+#include "url/origin.h"
 
 namespace {
 
@@ -356,4 +358,32 @@
   EXPECT_EQ(shared_link, ReadTextContent(web_contents, "link"));
 }
 
+// Regression test for crbug.com/40061291: the navigation to a share target has
+// to be attributed to the app being shared to. The action is verified to be in
+// the app's scope but may redirect anywhere, so an unattributed navigation --
+// which the network stack treats like a user-typed URL -- would let the app
+// make the browser send SameSite=Strict/Lax cookies to an arbitrary cross-site
+// URL.
+IN_PROC_BROWSER_TEST_F(WebShareTargetBrowserTest,
+                       ShareTargetNavigationIsAttributedToApp) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  const GURL app_url =
+      embedded_test_server()->GetURL("/web_share_target/poster.html");
+  const webapps::AppId app_id =
+      web_app::InstallWebAppFromManifest(browser(), app_url);
+
+  content::TestNavigationObserver navigation_observer(share_target_url());
+  navigation_observer.StartWatchingNewWebContents();
+
+  apps::IntentPtr intent = apps_util::MakeShareIntent(
+      /*text=*/"https://example.org/a",
+      /*title=*/"Hyperlink");
+
+  LaunchAppWithIntent(app_id, std::move(intent), share_target_url());
+  navigation_observer.Wait();
+
+  EXPECT_EQ(url::Origin::Create(share_target_url()),
+            navigation_observer.last_initiator_origin());
+}
+
 }  // namespace web_app

diff --git a/chrome/browser/ui/web_applications/web_share_target_browsertest.cc b/chrome/browser/ui/web_applications/web_share_target_browsertest.cc
index 4309787..24ea08c 100644
--- a/chrome/browser/ui/web_applications/web_share_target_browsertest.cc
+++ b/chrome/browser/ui/web_applications/web_share_target_browsertest.cc
@@ -42,12 +42,14 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/test_navigation_observer.h"
 #include "net/base/filename_util.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "services/network/public/cpp/resource_request_body.h"
 #include "storage/browser/file_system/file_system_context.h"
 #include "ui/display/types/display_constants.h"
 #include "url/gurl.h"
+#include "url/origin.h"
 
 namespace {
 
@@ -356,4 +358,32 @@
   EXPECT_EQ(shared_link, ReadTextContent(web_contents, "link"));
 }
 
+// Regression test for crbug.com/40061291: the navigation to a share target has
+// to be attributed to the app being shared to. The action is verified to be in
+// the app's scope but may redirect anywhere, so an unattributed navigation --
+// which the network stack treats like a user-typed URL -- would let the app
+// make the browser send SameSite=Strict/Lax cookies to an arbitrary cross-site
+// URL.
+IN_PROC_BROWSER_TEST_F(WebShareTargetBrowserTest,
+                       ShareTargetNavigationIsAttributedToApp) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  const GURL app_url =
+      embedded_test_server()->GetURL("/web_share_target/poster.html");
+  const webapps::AppId app_id =
+      web_app::InstallWebAppFromManifest(browser(), app_url);
+
+  content::TestNavigationObserver navigation_observer(share_target_url());
+  navigation_observer.StartWatchingNewWebContents();
+
+  apps::IntentPtr intent = apps_util::MakeShareIntent(
+      /*text=*/"https://example.org/a",
+      /*title=*/"Hyperlink");
+
+  LaunchAppWithIntent(app_id, std::move(intent), share_target_url());
+  navigation_observer.Wait();
+
+  EXPECT_EQ(url::Origin::Create(share_target_url()),
+            navigation_observer.last_initiator_origin());
+}
+
 }  // namespace web_app