From b0488a6721e2e1c4777b8111556bef48c26c0f4f Mon Sep 17 00:00:00 2001
From: Mohamed Adel <[email protected]>
Date: Mon, 21 Sep 2026 15:26:53 -0700
Subject: [PATCH] eCCT: Ensure isolated OTR profiles can't affect the incognito profile

Ensure that eCCT doesn't leak into the regular incognito profile or
crash the browser:
- Block creating popup windows for non-primary OTR profiles in
  PopupCreatorImpl and ChromeAndroidTaskTrackerImpl, destroying
  WebContents safely when popup creation is disallowed.
- In CustomTabIncognitoManager, avoid destroying the OTR profile during
  activity recreation, and only register in IncognitoTabHostRegistry
  when the CCT uses the primary OTR profile.
- Adjust ReparentingTask and ReparentingTabsTask to respect incognito
  branding.

TAG=agy

Bug: 518065595
Change-Id: I986bf4937e9de8169721596b33b0ed216df38cce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8320249
Reviewed-by: Moe Adel <[email protected]>
Commit-Queue: Moe Adel <[email protected]>
Reviewed-by: Jack Thiesen <[email protected]>
Reviewed-by: Jinsuk Kim <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1702116}
---
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/app/tab_activity_glue/PopupCreatorImpl.java b/chrome/android/java/src/org/chromium/chrome/browser/app/tab_activity_glue/PopupCreatorImpl.java
index a0dc45a..09438fa 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/app/tab_activity_glue/PopupCreatorImpl.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/app/tab_activity_glue/PopupCreatorImpl.java
@@ -98,6 +98,14 @@
             @Nullable WindowFeatures windowFeatures,
             @Nullable Bundle additionalIntentExtras,
             @Nullable Bundle startActivityOptions) {
+        // Only allow popups for the main incognito profile. Other OTR profiles
+        // cannot have popups. The caller transfers ownership of webContents to
+        // this method, so if popup creation fails we must destroy webContents.
+        if (profile.isOffTheRecord() && !profile.isPrimaryOtrProfile()) {
+            webContents.destroy();
+            return false;
+        }
+
         TabDelegateFactory delegateFactory = CustomTabDelegateFactory.createEmpty();
         Tab tab = TabBuilder.createDetachedSpareTab(context, delegateFactory, profile, webContents);
 
@@ -124,6 +132,13 @@
             return sMoveTabToNewPopupResultForTesting;
         }
 
+        Profile profile = tab.getProfile();
+        // Only allow popups for the main incognito profile. Other OTR profiles
+        // cannot have popups.
+        if (profile != null && profile.isOffTheRecord() && !profile.isPrimaryOtrProfile()) {
+            return false;
+        }
+
         Bundle optionsBundle =
                 resolveStartActivityOptions(null, windowFeatures, tab.getWindowAndroid());
 
@@ -487,7 +502,7 @@
 
     private static Intent createTrustedPopupIntent(
             @Nullable WindowFeatures windowFeatures,
-            boolean isIncognito,
+            boolean isIncognitoBranded,
             @Nullable Bundle additionalIntentExtras) {
         Intent intent = new Intent();
         intent.setClass(ContextUtils.getApplicationContext(), CustomTabActivity.class);
@@ -499,7 +514,7 @@
         if (additionalIntentExtras != null) {
             intent.putExtras(additionalIntentExtras);
         }
-        if (isIncognito) {
+        if (isIncognitoBranded) {
             IncognitoCustomTabIntentDataProvider.addIncognitoExtrasForChromeFeatures(
                     intent, IncognitoCctCallerId.CONTEXTUAL_POPUP);
         }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/app/tab_activity_glue/ReparentingTabsTask.java b/chrome/android/java/src/org/chromium/chrome/browser/app/tab_activity_glue/ReparentingTabsTask.java
index 8b3c531..b7b21129 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/app/tab_activity_glue/ReparentingTabsTask.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/app/tab_activity_glue/ReparentingTabsTask.java
@@ -117,7 +117,7 @@
 
         if (mTabs == null || mTabs.isEmpty()) return;
 
-        boolean isIncognito = mTabs.get(0).isIncognito();
+        boolean isIncognito = mTabs.get(0).isIncognitoBranded();
 
         for (Tab tab : mTabs) {
             AsyncTabParamsManagerSingleton.getInstance()
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/app/tab_activity_glue/ReparentingTask.java b/chrome/android/java/src/org/chromium/chrome/browser/app/tab_activity_glue/ReparentingTask.java
index d5e24331..1664bc5 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/app/tab_activity_glue/ReparentingTask.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/app/tab_activity_glue/ReparentingTask.java
@@ -137,7 +137,7 @@
         if (TextUtils.isEmpty(intent.getDataString())) {
             intent.setData(Uri.parse(mTab.getUrl().getSpec()));
         }
-        if (mTab.isIncognito()) {
+        if (mTab.isIncognitoBranded()) {
             intent.putExtra(
                     Browser.EXTRA_APPLICATION_ID,
                     ContextUtils.getApplicationContext().getPackageName());
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIncognitoManager.java b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIncognitoManager.java
index 35e99017..572835b 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIncognitoManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabIncognitoManager.java
@@ -4,7 +4,6 @@
 
 package org.chromium.chrome.browser.customtabs;
 
-import static org.chromium.build.NullUtil.assumeNonNull;
 import static org.chromium.chrome.browser.flags.CustomTabProfileType.INCOGNITO;
 
 import android.app.Activity;
@@ -23,6 +22,7 @@
 import org.chromium.chrome.browser.profiles.ProfileProvider;
 import org.chromium.chrome.browser.tabmodel.IncognitoTabHost;
 import org.chromium.chrome.browser.tabmodel.IncognitoTabHostRegistry;
+import org.chromium.chrome.browser.tabmodel.IncognitoTabHostUtils;
 
 /**
  * Implements incognito tab host for the given instance of Custom Tab activity. This class exists
@@ -65,16 +65,35 @@
             IncognitoTabHostRegistry.getInstance().unregister(mIncognitoTabHost);
         }
 
-        Profile otrProfile = assumeNonNull(mProfileProviderSupplier.get()).getOffTheRecordProfile();
-        if (otrProfile != null) {
+        // Do not destroy the profile if the activity is just being recreated (e.g. on
+        // configuration change), because the tab and its profile will be reused.
+        if (!mActivity.isFinishing()) {
+            return;
+        }
+
+        ProfileProvider profileProvider = mProfileProviderSupplier.get();
+        if (profileProvider == null) return;
+        Profile otrProfile = profileProvider.getOffTheRecordProfile();
+        if (otrProfile != null
+                && (!otrProfile.isPrimaryOtrProfile()
+                        || (!IncognitoTabHostUtils.doIncognitoTabsExist()
+                                && !IncognitoTabHostUtils.isIncognitoTabModelActive()))) {
             ProfileManager.destroyWhenAppropriate(otrProfile);
         }
     }
 
     private void initializeIncognito() {
+        // Only register in IncognitoTabHostRegistry if this CCT shares Chrome's primary_otr
+        // profile. Isolated CCT:Incognito profiles must not register as they would prevent
+        // Chrome's primary incognito profile from being cleaned up when main incognito tabs close.
         if (mIntentDataProvider.isOpenedByChrome()) {
-            mIncognitoTabHost = new IncognitoCustomTabHost();
-            IncognitoTabHostRegistry.getInstance().register(mIncognitoTabHost);
+            ProfileProvider profileProvider = mProfileProviderSupplier.get();
+            Profile otrProfile =
+                    profileProvider != null ? profileProvider.getOffTheRecordProfile() : null;
+            if (otrProfile == null || otrProfile.isPrimaryOtrProfile()) {
+                mIncognitoTabHost = new IncognitoCustomTabHost();
+                IncognitoTabHostRegistry.getInstance().register(mIncognitoTabHost);
+            }
         }
 
         maybeCreateIncognitoTabSnapshotController();
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityEphemeralTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityEphemeralTest.java
index b816477..3a89c3d 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityEphemeralTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityEphemeralTest.java
@@ -46,6 +46,7 @@
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.library_loader.LibraryLoader;
 import org.chromium.base.supplier.OneshotSupplier;
+import org.chromium.base.test.util.ApplicationTestUtils;
 import org.chromium.base.test.util.Batch;
 import org.chromium.base.test.util.CallbackHelper;
 import org.chromium.base.test.util.CommandLineFlags;
@@ -55,8 +56,10 @@
 import org.chromium.base.test.util.HistogramWatcher;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.IntentHandler;
+import org.chromium.chrome.browser.app.tab_activity_glue.PopupCreatorImpl;
 import org.chromium.chrome.browser.browserservices.intents.BrowserServicesIntentDataProvider;
 import org.chromium.chrome.browser.browserservices.intents.SessionHolder;
+import org.chromium.chrome.browser.content.WebContentsFactory;
 import org.chromium.chrome.browser.customtabs.features.toolbar.CustomTabToolbar;
 import org.chromium.chrome.browser.firstrun.FirstRunStatus;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
@@ -78,6 +81,8 @@
 import org.chromium.components.browser_ui.styles.ChromeColors;
 import org.chromium.components.page_info.PageInfoController;
 import org.chromium.components.user_prefs.UserPrefs;
+import org.chromium.content_public.browser.WebContents;
+import org.chromium.content_public.browser.test.util.DOMUtils;
 import org.chromium.net.test.EmbeddedTestServerRule;
 
 import java.util.concurrent.TimeoutException;
@@ -396,4 +401,63 @@
         launchEphemeralCustomTabActivity();
         histogramWatcher.assertExpected();
     }
+
+    @Test
+    @MediumTest
+    public void testPopupCreationBlockedAndDoesNotCrashOnClose() throws Exception {
+        PopupCreatorImpl.setSetMovableTaskRequiredForPopupsForTesting(false);
+        String popupPageUrl =
+                mEmbeddedTestServerRule
+                        .getServer()
+                        .getURL("/chrome/test/data/android/popup_on_click.html");
+        Intent intent =
+                createMinimalCustomTabIntent(
+                                ApplicationProvider.getApplicationContext(), popupPageUrl)
+                        .putExtra(EXTRA_ENABLE_EPHEMERAL_BROWSING, true);
+        mCustomTabActivityTestRule.startCustomTabActivityWithIntent(intent);
+        CustomTabActivity activity = mCustomTabActivityTestRule.getActivity();
+        Profile ephemeralProfile =
+                ThreadUtils.runOnUiThreadBlocking(() -> activity.getActivityTab().getProfile());
+        assertTrue(ephemeralProfile.isOffTheRecord());
+        assertFalse(ephemeralProfile.isPrimaryOtrProfile());
+
+        HistogramWatcher popupHistogram =
+                HistogramWatcher.newSingleRecordWatcher(
+                        "Android.MultiWindowMode.PopupOpensInNewWindow", false);
+
+        DOMUtils.clickNode(activity.getActivityTab().getWebContents(), "link_with_bounds");
+
+        CriteriaHelper.pollUiThread(
+                () -> {
+                    Criteria.checkThat(
+                            activity.getTabModelSelector().getCurrentModel().getCount(),
+                            Matchers.is(2));
+                });
+        popupHistogram.assertExpected();
+
+        Profile newTabProfile =
+                ThreadUtils.runOnUiThreadBlocking(() -> activity.getActivityTab().getProfile());
+        assertEquals(ephemeralProfile, newTabProfile);
+
+        ThreadUtils.runOnUiThreadBlocking(
+                () -> {
+                    WebContents webContents =
+                            WebContentsFactory.createWebContents(
+                                    ephemeralProfile,
+                                    /* initiallyHidden= */ false,
+                                    /* initializeRenderer= */ true);
+                    assertFalse(
+                            new PopupCreatorImpl()
+                                    .createNewPopupFromWebContents(
+                                            activity,
+                                            ephemeralProfile,
+                                            webContents,
+                                            null,
+                                            null,
+                                            null));
+                    assertTrue(webContents.isDestroyed());
+                });
+
+        ApplicationTestUtils.finishActivity(activity);
+    }
 }
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/app/tab_activity_glue/PopupCreatorImplUnitTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/app/tab_activity_glue/PopupCreatorImplUnitTest.java
index c5fdbbaf..ccb3eb06 100644
--- a/chrome/android/junit/src/org/chromium/chrome/browser/app/tab_activity_glue/PopupCreatorImplUnitTest.java
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/app/tab_activity_glue/PopupCreatorImplUnitTest.java
@@ -358,6 +358,106 @@
                 IntentUtils.isTrustedIntentFromSelf(sentIntent));
     }
 
+    @Test
+    public void testIntentParams_twaOpener_invalidSessionId() {
+        BaseCustomTabActivity twaActivity = mock(BaseCustomTabActivity.class);
+        BrowserServicesIntentDataProvider provider = mock(BrowserServicesIntentDataProvider.class);
+        doReturn(twaActivity).when(mTab).getContext();
+        doReturn(provider).when(twaActivity).getIntentDataProvider();
+
+        when(provider.getActivityType()).thenReturn(ActivityType.TRUSTED_WEB_ACTIVITY);
+        when(provider.getClientPackageName()).thenReturn("org.chromium.test.twa");
+        when(provider.getUrlToLoad()).thenReturn("https://example.com/twa");
+
+        Intent sourceIntent = new Intent();
+        sourceIntent.putExtra(CustomTabsIntent.EXTRA_SESSION_ID, new Intent("malicious_action"));
+        when(provider.getIntent()).thenReturn(sourceIntent);
+
+        final WindowFeatures windowFeatures = new WindowFeatures(12, 34, 56, null);
+        mPopupCreator.moveTabToNewPopup(mTab, windowFeatures);
+
+        final ArgumentCaptor<Intent> captor = ArgumentCaptor.forClass(Intent.class);
+        verify(mReparentingTask).begin(any(), captor.capture(), any(), any());
+        final Intent sentIntent = captor.getValue();
+
+        assertFalse(
+                "Invalid EXTRA_SESSION_ID must not be forwarded",
+                sentIntent.hasExtra(CustomTabsIntent.EXTRA_SESSION_ID));
+    }
+
+    @Test
+    public void testIntentParams_standardCctOpener() {
+        BaseCustomTabActivity cctActivity = mock(BaseCustomTabActivity.class);
+        BrowserServicesIntentDataProvider provider = mock(BrowserServicesIntentDataProvider.class);
+        doReturn(cctActivity).when(mTab).getContext();
+        doReturn(provider).when(cctActivity).getIntentDataProvider();
+
+        when(provider.getActivityType()).thenReturn(ActivityType.CUSTOM_TAB);
+        when(provider.getClientPackageName()).thenReturn("org.chromium.test.cct");
+        when(provider.getUrlToLoad()).thenReturn("https://example.com/cct");
+        List<String> origins = Arrays.asList("https://trusted.example.com");
+        when(provider.getTrustedWebActivityAdditionalOrigins()).thenReturn(origins);
+
+        Intent sourceIntent = new Intent();
+        IBinder session = new Binder();
+        IntentUtils.safePutBinderExtra(sourceIntent, CustomTabsIntent.EXTRA_SESSION, session);
+        PendingIntent sessionId = mock(PendingIntent.class);
+        sourceIntent.putExtra(CustomTabsIntent.EXTRA_SESSION_ID, sessionId);
+        when(provider.getIntent()).thenReturn(sourceIntent);
+
+        final WindowFeatures windowFeatures = new WindowFeatures(12, 34, 56, null);
+        mPopupCreator.moveTabToNewPopup(mTab, windowFeatures);
+
+        final ArgumentCaptor<Intent> captor = ArgumentCaptor.forClass(Intent.class);
+        verify(mReparentingTask).begin(any(), captor.capture(), any(), any());
+        final Intent sentIntent = captor.getValue();
+
+        assertEquals(
+                new ComponentName(ContextUtils.getApplicationContext(), CustomTabActivity.class),
+                sentIntent.getComponent());
+        assertEquals(
+                CustomTabsUiType.POPUP,
+                sentIntent.getIntExtra(CustomTabIntentDataProvider.EXTRA_UI_TYPE, -1));
+        assertFalse(
+                "EXTRA_LAUNCH_AS_TRUSTED_WEB_ACTIVITY must not be set for standard CCT",
+                sentIntent.hasExtra(TrustedWebUtils.EXTRA_LAUNCH_AS_TRUSTED_WEB_ACTIVITY));
+        assertNull("Data URI must not be set for standard CCT popup", sentIntent.getData());
+        assertFalse(sentIntent.hasExtra(CustomTabsIntent.EXTRA_SESSION));
+        assertFalse(sentIntent.hasExtra(CustomTabsIntent.EXTRA_SESSION_ID));
+        assertFalse(sentIntent.hasExtra(IntentHandler.EXTRA_CALLING_ACTIVITY_PACKAGE));
+        assertFalse(
+                sentIntent.hasExtra(
+                        TrustedWebActivityIntentBuilder.EXTRA_ADDITIONAL_TRUSTED_ORIGINS));
+    }
+
+    @Test
+    public void testMoveTabToNewPopup_isolatedOtrProfile_returnsFalse() {
+        final WindowFeatures windowFeatures = new WindowFeatures(12, 34, 56, null);
+        Profile isolatedProfile = mock(Profile.class);
+        doReturn(true).when(isolatedProfile).isOffTheRecord();
+        doReturn(false).when(isolatedProfile).isPrimaryOtrProfile();
+        doReturn(isolatedProfile).when(mTab).getProfile();
+
+        assertFalse(
+                "moveTabToNewPopup should return false for isolated OTR profiles",
+                mPopupCreator.moveTabToNewPopup(mTab, windowFeatures));
+        verify(mReparentingTask, never()).begin(any(), any(), any(), any());
+    }
+
+    @Test
+    public void testCreateNewPopupFromWebContents_isolatedOtrProfile_returnsFalse() {
+        final WindowFeatures windowFeatures = new WindowFeatures(12, 34, 56, null);
+        Profile isolatedProfile = mock(Profile.class);
+        doReturn(true).when(isolatedProfile).isOffTheRecord();
+        doReturn(false).when(isolatedProfile).isPrimaryOtrProfile();
+
+        assertFalse(
+                "createNewPopupFromWebContents should return false for isolated OTR profiles",
+                mPopupCreator.createNewPopupFromWebContents(
+                        mContext, isolatedProfile, mWebContents, windowFeatures, null, null));
+        verify(mWebContents).destroy();
+    }
+
     private ActivityOptions getActivityOptionsPassedToReparentingTask() {
         ArgumentCaptor<Bundle> captor = ArgumentCaptor.forClass(Bundle.class);
         verify(mReparentingTask).begin(any(), any(), captor.capture(), any());
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/app/tab_activity_glue/ReparentingTaskUnitTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/app/tab_activity_glue/ReparentingTaskUnitTest.java
index fa384fc2e..d3f52878 100644
--- a/chrome/android/junit/src/org/chromium/chrome/browser/app/tab_activity_glue/ReparentingTaskUnitTest.java
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/app/tab_activity_glue/ReparentingTaskUnitTest.java
@@ -7,6 +7,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
@@ -69,7 +70,7 @@
     @Mock private WindowAndroid mWindowAndroid;
 
     @Mock private Tab mTab;
-    private final UserDataHost mUserDataHost = new UserDataHost();
+    private UserDataHost mUserDataHost;
     @Mock private ReparentingTask mTabReparentingTask;
     @Mock private WebContents mWebContents;
 
@@ -83,6 +84,7 @@
 
     @Before
     public void setup() {
+        mUserDataHost = new UserDataHost();
         when(mTab.getUserDataHost()).thenReturn(mUserDataHost);
         when(mTab.getWebContents()).thenReturn(mWebContents);
         when(mTab.getId()).thenReturn(TAB_ID);
... (file diff truncated)