From c04dac75898ba79c9f5de1d6e72611bf4629fa6c Mon Sep 17 00:00:00 2001
From: Jihad Hanna <[email protected]>
Date: Mon, 13 Jul 2026 23:26:46 -0700
Subject: [PATCH] Tighten origin handling for CredMan-triggered focus events

1) Early return when CredMan is showing (`kIsShowing`):
While the Android Credential Manager (CredMan) bottom sheet is showing
(`credman_sheet_status_ == CredManBottomSheetLifecycle::kIsShowing`),
OnAskForValuesToFill() and OnSelectControlSelectionChanged() drop
incoming IPCs to prevent a compromised renderer from spoofing the
session origin in the background while the user interacts with the UI.

OnFocusOnFormField() previously lacked this early return. If a focus IPC
arrived while CredMan was showing, ShouldShowCredManForField() returned
false, but execution proceeded to bridge_->OnFocusChanged(). This could
desynchronize C++ and Java state or surface an unexpected Autofill
dropdown while OnAskForValuesToFill() was ignored in C++. Adding an
early return in OnFocusOnFormField() freezes background form focus
interactions while CredMan is active.

2) Guard UpdateCurrentField() with `field_to_focus`:
When CredMan is triggered, OnFocusOnFormField() proactively updates
session_state_->current_field so that if OnAskForValuesToFill() is
subsequently ignored while the sheet is showing, the origin remains
accurate for the focused field.

Previously, UpdateCurrentField() ran unconditionally. If CredMan was
triggered from an unrelated form or cross-origin frame,
StartFocusChange() returned std::nullopt (`!IsLinkedForm(form)`), but
UpdateCurrentField() blindly overwrote session_state_->current_field
with the unrelated origin while leaving session_state_->form pointing to
the active session form. Because OnAskForValuesToFill() then dropped
the IPC (`kIsShowing`), StartNewSession() was never called. When
OnAutofillAvailable() later ran, it passed the poisoned origin as
`triggered_origin` to FormForest, allowing cross-origin credential
disclosure. Guarding UpdateCurrentField() with `if (field_to_focus)`
ensures proactive origin updates only occur when the focused field
belongs to the active session form.

3) Test modifications and additions:
- CredManEarlyReturnLeavesStaleCurrentFieldOrigin_Fixed is reworked into
CredManTriggerForUnrelatedFormPreservesSessionOrigin. The previous test
simulated focus on a separate, unrelated form. Under the new guard,
focusing an unrelated form (`field_to_focus == std::nullopt`) no longer
overwrites the active session origin. The test is updated to assert that
the session origin is preserved rather than overwritten.
- CredManTriggerForSessionFormFieldUpdatesOrigin is added to verify that
when a cross-origin field belonging to the active session form triggers
CredMan across a multi-frame form, UpdateCurrentField() still
proactively updates current_field.origin.
- CredManActiveBlocksSpoofedFocusOnFormField is added to verify that
OnFocusOnFormField() IPCs arriving while a CredMan sheet is active
(`kIsShowing`) are blocked and cannot mutate the session origin.
- CredManActiveBlocksSpoofed* also had their origin expectations changed
to null because now a fresh FocusOnFormField() that leads to CredMan
bottom sheet does not update the session_state_ at all.

Fixed: 523714535
Change-Id: I563984c579dc51e8f7b60155ba305c1d3dd81e3c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8084823
Reviewed-by: Jan Keitel <[email protected]>
Commit-Queue: Jihad Hanna <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1661639}
---

diff --git a/components/android_autofill/browser/android_autofill_provider.cc b/components/android_autofill/browser/android_autofill_provider.cc
index 3bebddfb..81601646 100644
--- a/components/android_autofill/browser/android_autofill_provider.cc
+++ b/components/android_autofill/browser/android_autofill_provider.cc
@@ -596,6 +596,15 @@
     const FormData& form,
     const FormFieldData& field) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (credman_sheet_status_ == CredManBottomSheetLifecycle::kIsShowing) {
+    // While CredMan is active, the user cannot legitimately interact with the
+    // page. We ignore this request to prevent a compromised renderer from
+    // spoofing the session origin (overwriting `current_field`) in the
+    // background. We preserve the session state that triggered CredMan so that
+    // the subsequent fill goes to the correct frame. If the user dismisses
+    // CredMan, a new session will be started on the next focus event.
+    return;
+  }
   std::optional<FieldInfo> field_to_focus = StartFocusChange(form, field);
   if (content::RenderFrameHost* rfh =
           GetRenderFrameHost(manager, field.host_frame());
@@ -605,7 +614,9 @@
     // subsequent `OnAskForValuesToFill()` IPC will be ignored while CredMan is
     // showing (to block spoofing), we must set the correct origin now before
     // the block takes effect, otherwise the session will retain a stale origin.
-    UpdateCurrentField(manager, form, field);
+    if (field_to_focus) {
+      UpdateCurrentField(manager, form, field);
+    }
     return;  // The focus event will be completed after CredMan closes.
   }
   if (field_to_focus) {
diff --git a/components/android_autofill/browser/android_autofill_provider_unittest.cc b/components/android_autofill/browser/android_autofill_provider_unittest.cc
index cb2eedd..68633e75 100644
--- a/components/android_autofill/browser/android_autofill_provider_unittest.cc
+++ b/components/android_autofill/browser/android_autofill_provider_unittest.cc
@@ -1140,12 +1140,12 @@
   FocusSubFrameFormField(sub_frame_webauthn_email_field());
 }
 
-// Tests that when CredMan is triggered, the current field's origin is updated
-// proactively to the frame origin of the passkey field, ensuring we don't use
-// a stale origin if CredMan is dismissed and autofill completes.
-// (see crbug.com/518084475).
+// Tests that when CredMan is triggered for a field in an unrelated form
+// (different from the active session's form), the session origin is preserved
+// rather than overwritten with the unrelated form's origin. (see
+// crbug.com/523714535, crbug.com/518084475).
 TEST_F(AndroidAutofillProviderWithCredManMultiFrameTest,
-       CredManEarlyReturnLeavesStaleCurrentFieldOrigin_Fixed) {
+       CredManTriggerForUnrelatedFormPreservesSessionOrigin) {
   // 1. Start session on main frame (origin https://foo.com).
   android_autofill_manager().OnFormsSeen({test_form()}, {});
   // Focus main frame field to start session and set origin to foo.com.
@@ -1156,23 +1156,111 @@
   ASSERT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
             url::Origin::Create(GURL("https://foo.com")));
 
-  // 2. Focus subframe field (origin https://bar.com) which triggers CredMan.
-  // Expect CredMan to be triggered on subframe.
+  // 2. Focus subframe field (origin https://bar.com) on an unrelated form,
+  // which triggers CredMan. Expect CredMan to be triggered on subframe.
   EXPECT_CALL(*sub_frame_mock_delegate_, TriggerCredManUi);
 
-  // Simulate Focus FIRST (which should update origin to bar.com).
+  // Simulate Focus FIRST (which should NOT update origin to bar.com because the
+  // field does not belong to the active session form).
   android_autofill_manager().SimulateOnFocusOnFormField(
       sub_frame_test_form(), sub_frame_webauthn_email_field());
   EXPECT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
-            url::Origin::Create(GURL("https://bar.com")));
+            url::Origin::Create(GURL("https://foo.com")));
 
   // Simulate AskForValuesToFill() SECOND (which returns early because CredMan
-  // is showing) and verify origin is STILL bar.com (not reverted or stale
-  // foo.com).
+  // is showing) and verify origin is STILL foo.com (not overwritten by
+  // bar.com).
   android_autofill_manager().SimulateOnAskForValuesToFill(
       sub_frame_test_form(), sub_frame_webauthn_email_field());
   EXPECT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
-            url::Origin::Create(GURL("https://bar.com")));
+            url::Origin::Create(GURL("https://foo.com")));
+}
+
+// Tests that when CredMan is triggered for a field that belongs to the active
+// session's form (even in a different frame of a multi-frame form), the current
+// field's origin is proactively updated to that field's origin.
+// (see crbug.com/518084475).
+TEST_F(AndroidAutofillProviderWithCredManMultiFrameTest,
+       CredManTriggerForSessionFormFieldUpdatesOrigin) {
+  const url::Origin foo_origin = url::Origin::Create(GURL("https://foo.com"));
+  const url::Origin bar_origin = url::Origin::Create(GURL("https://bar.com"));
+
+  FormData multi_frame_form = test::GetFormData({
+      .fields = {{
+                     .host_frame =
+                         LocalFrameToken(main_frame()->GetFrameToken().value()),
+                     .origin = foo_origin,
+                 },
+                 {
+                     .host_frame =
+                         LocalFrameToken(sub_frame_->GetFrameToken().value()),
+                     .autocomplete_attribute = "webauthn",
+                     .origin = bar_origin,
+                 }},
+      .url = "https://foo.com/form.html",
+      .main_frame_origin = foo_origin,
+  });
+
+  const FormFieldData& foo_field = multi_frame_form.fields()[0];
+  const FormFieldData& bar_field = multi_frame_form.fields()[1];
+
+  android_autofill_manager().OnFormsSeen({multi_frame_form}, {});
+
+  // 1. Start session on foo_field (origin https://foo.com).
+  android_autofill_manager().SimulateOnAskForValuesToFill(multi_frame_form,
+                                                          foo_field);
+  android_autofill_manager().SimulateOnFocusOnFormField(multi_frame_form,
+                                                        foo_field);
+  ASSERT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
+            foo_origin);
+
+  // 2. Focus bar_field (origin https://bar.com) which is part of the SAME form
+  // and triggers CredMan.
+  EXPECT_CALL(*sub_frame_mock_delegate_, TriggerCredManUi);
+
+  // Simulate Focus FIRST (which should proactively update origin to bar.com).
+  android_autofill_manager().SimulateOnFocusOnFormField(multi_frame_form,
+                                                        bar_field);
+  EXPECT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
+            bar_origin);
+
+  // Simulate AskForValuesToFill() SECOND (which returns early because CredMan
+  // is showing) and verify origin is STILL bar.com.
+  android_autofill_manager().SimulateOnAskForValuesToFill(multi_frame_form,

diff --git a/components/android_autofill/browser/android_autofill_provider_unittest.cc b/components/android_autofill/browser/android_autofill_provider_unittest.cc
index cb2eedd..68633e75 100644
--- a/components/android_autofill/browser/android_autofill_provider_unittest.cc
+++ b/components/android_autofill/browser/android_autofill_provider_unittest.cc
@@ -1140,12 +1140,12 @@
   FocusSubFrameFormField(sub_frame_webauthn_email_field());
 }
 
-// Tests that when CredMan is triggered, the current field's origin is updated
-// proactively to the frame origin of the passkey field, ensuring we don't use
-// a stale origin if CredMan is dismissed and autofill completes.
-// (see crbug.com/518084475).
+// Tests that when CredMan is triggered for a field in an unrelated form
+// (different from the active session's form), the session origin is preserved
+// rather than overwritten with the unrelated form's origin. (see
+// crbug.com/523714535, crbug.com/518084475).
 TEST_F(AndroidAutofillProviderWithCredManMultiFrameTest,
-       CredManEarlyReturnLeavesStaleCurrentFieldOrigin_Fixed) {
+       CredManTriggerForUnrelatedFormPreservesSessionOrigin) {
   // 1. Start session on main frame (origin https://foo.com).
   android_autofill_manager().OnFormsSeen({test_form()}, {});
   // Focus main frame field to start session and set origin to foo.com.
@@ -1156,23 +1156,111 @@
   ASSERT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
             url::Origin::Create(GURL("https://foo.com")));
 
-  // 2. Focus subframe field (origin https://bar.com) which triggers CredMan.
-  // Expect CredMan to be triggered on subframe.
+  // 2. Focus subframe field (origin https://bar.com) on an unrelated form,
+  // which triggers CredMan. Expect CredMan to be triggered on subframe.
   EXPECT_CALL(*sub_frame_mock_delegate_, TriggerCredManUi);
 
-  // Simulate Focus FIRST (which should update origin to bar.com).
+  // Simulate Focus FIRST (which should NOT update origin to bar.com because the
+  // field does not belong to the active session form).
   android_autofill_manager().SimulateOnFocusOnFormField(
       sub_frame_test_form(), sub_frame_webauthn_email_field());
   EXPECT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
-            url::Origin::Create(GURL("https://bar.com")));
+            url::Origin::Create(GURL("https://foo.com")));
 
   // Simulate AskForValuesToFill() SECOND (which returns early because CredMan
-  // is showing) and verify origin is STILL bar.com (not reverted or stale
-  // foo.com).
+  // is showing) and verify origin is STILL foo.com (not overwritten by
+  // bar.com).
   android_autofill_manager().SimulateOnAskForValuesToFill(
       sub_frame_test_form(), sub_frame_webauthn_email_field());
   EXPECT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
-            url::Origin::Create(GURL("https://bar.com")));
+            url::Origin::Create(GURL("https://foo.com")));
+}
+
+// Tests that when CredMan is triggered for a field that belongs to the active
+// session's form (even in a different frame of a multi-frame form), the current
+// field's origin is proactively updated to that field's origin.
+// (see crbug.com/518084475).
+TEST_F(AndroidAutofillProviderWithCredManMultiFrameTest,
+       CredManTriggerForSessionFormFieldUpdatesOrigin) {
+  const url::Origin foo_origin = url::Origin::Create(GURL("https://foo.com"));
+  const url::Origin bar_origin = url::Origin::Create(GURL("https://bar.com"));
+
+  FormData multi_frame_form = test::GetFormData({
+      .fields = {{
+                     .host_frame =
+                         LocalFrameToken(main_frame()->GetFrameToken().value()),
+                     .origin = foo_origin,
+                 },
+                 {
+                     .host_frame =
+                         LocalFrameToken(sub_frame_->GetFrameToken().value()),
+                     .autocomplete_attribute = "webauthn",
+                     .origin = bar_origin,
+                 }},
+      .url = "https://foo.com/form.html",
+      .main_frame_origin = foo_origin,
+  });
+
+  const FormFieldData& foo_field = multi_frame_form.fields()[0];
+  const FormFieldData& bar_field = multi_frame_form.fields()[1];
+
+  android_autofill_manager().OnFormsSeen({multi_frame_form}, {});
+
+  // 1. Start session on foo_field (origin https://foo.com).
+  android_autofill_manager().SimulateOnAskForValuesToFill(multi_frame_form,
+                                                          foo_field);
+  android_autofill_manager().SimulateOnFocusOnFormField(multi_frame_form,
+                                                        foo_field);
+  ASSERT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
+            foo_origin);
+
+  // 2. Focus bar_field (origin https://bar.com) which is part of the SAME form
+  // and triggers CredMan.
+  EXPECT_CALL(*sub_frame_mock_delegate_, TriggerCredManUi);
+
+  // Simulate Focus FIRST (which should proactively update origin to bar.com).
+  android_autofill_manager().SimulateOnFocusOnFormField(multi_frame_form,
+                                                        bar_field);
+  EXPECT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
+            bar_origin);
+
+  // Simulate AskForValuesToFill() SECOND (which returns early because CredMan
+  // is showing) and verify origin is STILL bar.com.
+  android_autofill_manager().SimulateOnAskForValuesToFill(multi_frame_form,
+                                                          bar_field);
+  EXPECT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
+            bar_origin);
+}
+
+// Tests that a compromised renderer cannot spoof the session origin by sending
+// a malicious FocusOnFormField() IPC while a CredMan sheet is active.
+// (see crbug.com/523714535).
+TEST_F(AndroidAutofillProviderWithCredManMultiFrameTest,
+       CredManActiveBlocksSpoofedFocusOnFormField) {
+  // 1. Start session on main frame (origin https://foo.com) and trigger
+  // CredMan.
+  android_autofill_manager().OnFormsSeen({test_form()}, {});
+  EXPECT_CALL(cred_man_delegate(), TriggerCredManUi);
+  android_autofill_manager().SimulateOnFocusOnFormField(test_form(),
+                                                        webauthn_email_field());
+  android_autofill_manager().SimulateOnAskForValuesToFill(
+      test_form(), webauthn_email_field());
+  // Because test_form() was not linked when CredMan opened on this fresh page,
+  // UpdateCurrentField() is skipped to prevent cross-origin poisoning, so
+  // last_focused_field_origin() remains uninitialized right while CredMan is
+  // showing.
+  ASSERT_TRUE(
+      test_api(autofill_provider()).last_focused_field_origin().opaque());
+  ASSERT_EQ(test_api(autofill_provider()).form(), nullptr);
+
+  // 2. Spoof FocusOnFormField() from attacker.com: Attacker sends fake
+  // FocusOnFormField() while CredMan is showing. Verify that origin remains
+  // unchanged and that the spoof is blocked.
+  android_autofill_manager().SimulateOnFocusOnFormField(
+      sub_frame_test_form(), sub_frame_webauthn_email_field());
+  EXPECT_TRUE(
+      test_api(autofill_provider()).last_focused_field_origin().opaque());
+  EXPECT_EQ(test_api(autofill_provider()).form(), nullptr);
 }
 
 // Tests that a compromised renderer cannot spoof the session origin by sending
@@ -1188,16 +1276,18 @@
                                                         webauthn_email_field());
   android_autofill_manager().SimulateOnAskForValuesToFill(
       test_form(), webauthn_email_field());
-  ASSERT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
-            url::Origin::Create(GURL("https://foo.com")));
+  ASSERT_TRUE(
+      test_api(autofill_provider()).last_focused_field_origin().opaque());
+  ASSERT_EQ(test_api(autofill_provider()).form(), nullptr);
 
   // 2. Spoof AskForValuesToFill() from attacker.com: Attacker sends fake
   // AskForValuesToFill() while CredMan is showing. Verify that origin remains
-  // foo.com and that the spoof is blocked.
+  // unchanged and that the spoof is blocked.
   android_autofill_manager().SimulateOnAskForValuesToFill(
       sub_frame_test_form(), sub_frame_webauthn_email_field());
-  EXPECT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
-            url::Origin::Create(GURL("https://foo.com")));
+  EXPECT_TRUE(
+      test_api(autofill_provider()).last_focused_field_origin().opaque());
+  EXPECT_EQ(test_api(autofill_provider()).form(), nullptr);
 }
 
 // Tests that a compromised renderer cannot spoof the session origin by sending
@@ -1216,19 +1306,21 @@
                                                         webauthn_email_field());
   android_autofill_manager().SimulateOnAskForValuesToFill(
       test_form(), webauthn_email_field());
-  ASSERT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
-            url::Origin::Create(GURL("https://foo.com")));
+  ASSERT_TRUE(
+      test_api(autofill_provider()).last_focused_field_origin().opaque());
+  ASSERT_EQ(test_api(autofill_provider()).form(), nullptr);
 
   // 2. Spoof SelectControlSelectionChanged() from attacker.com: Attacker sends
   // fake SelectControlSelectionChanged() while CredMan is showing. Verify that
-  // origin remains foo.com and that the spoof is blocked.
+  // origin remains unchanged and that the spoof is blocked.
   autofill_provider().OnSelectControlSelectionChanged(
       &android_autofill_manager(), sub_frame_test_form(),
       sub_frame_webauthn_email_field());
 
-  // Verify origin remains foo.com (spoof blocked!).
-  EXPECT_EQ(test_api(autofill_provider()).last_focused_field_origin(),
-            url::Origin::Create(GURL("https://foo.com")));
+  // Verify origin remains unchanged (spoof blocked!).
+  EXPECT_TRUE(
+      test_api(autofill_provider()).last_focused_field_origin().opaque());
+  EXPECT_EQ(test_api(autofill_provider()).form(), nullptr);
 }
 
 class AndroidAutofillProviderCredManSpoofSheetStatusTest