CVE-2026-15124
Overview
Files Changed
chrome/browser/keyboard_accessory/android/password_accessory_controller_impl.ccchrome/browser/keyboard_accessory/android/password_accessory_controller_impl_unittest.ccchrome/browser/password_manager/chrome_password_manager_client.ccchrome/browser/password_manager/chrome_password_manager_client.hchrome/browser/password_manager/chrome_password_manager_client_unittest.cc
Patch
From 64e2f47453fbcb87cb0182d69e8c9a5ba6e27099 Mon Sep 17 00:00:00 2001 From: Jan Keitel <[email protected]> Date: Thu, 25 Jun 2026 06:46:30 -0700 Subject: [PATCH] Let PasswordManagerClient::IsSavingAndFillingEnabled take url::Origin This change transitions PasswordManagerClient::IsSavingAndFillingEnabled from taking a GURL to taking a url::Origin. This is a security improvement to prevent same-site sandboxed iframes (which have opaque origins) from saving or filling credentials. A 1-argument non-virtual helper is added to PasswordManagerClient which delegates to the 2-argument virtual method. The virtual method no longer has default parameters. Derived classes (ChromePasswordManagerClient, IOSChromePasswordManagerClient, and StubPasswordManagerClient) are updated to override the new signature. Call sites and test mocks are updated to pass url::Origin. Bug: 523735038 TAG=agy CONV=068598ea-4183-4393-ad52-c2807273ab47 Change-Id: If213675c3cd35448ae5c38e414e845e2f450d64b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7978680 Commit-Queue: Jan Keitel <[email protected]> Reviewed-by: Christoph Schwering <[email protected]> Reviewed-by: Rohit Rao <[email protected]> Reviewed-by: Maria Kazinova <[email protected]> Auto-Submit: Jan Keitel <[email protected]> Cr-Commit-Position: refs/heads/main@{#1652381} --- diff --git a/chrome/browser/keyboard_accessory/android/password_accessory_controller_impl.cc b/chrome/browser/keyboard_accessory/android/password_accessory_controller_impl.cc index d2da4ed..f53db4f 100644 --- a/chrome/browser/keyboard_accessory/android/password_accessory_controller_impl.cc +++ b/chrome/browser/keyboard_accessory/android/password_accessory_controller_impl.cc @@ -664,7 +664,7 @@ bool PasswordAccessoryControllerImpl::ShouldShowRecoveryToggle( const url::Origin& origin) const { - return password_client_->IsSavingAndFillingEnabled(origin.GetURL()); + return password_client_->IsSavingAndFillingEnabled(origin); } base::WeakPtr<ManualFillingController> diff --git a/chrome/browser/keyboard_accessory/android/password_accessory_controller_impl_unittest.cc b/chrome/browser/keyboard_accessory/android/password_accessory_controller_impl_unittest.cc index a6b33f2..11e8113 100644 --- a/chrome/browser/keyboard_accessory/android/password_accessory_controller_impl_unittest.cc +++ b/chrome/browser/keyboard_accessory/android/password_accessory_controller_impl_unittest.cc @@ -168,7 +168,7 @@ MOCK_METHOD(bool, IsSavingAndFillingEnabled, - (const GURL&), + (const url::Origin&, base::optional_ref<const GURL>), (const, override)); MOCK_METHOD(std::unique_ptr<device_reauth::DeviceAuthenticator>, @@ -934,7 +934,8 @@ cache()->SaveCredentialsAndBlocklistedForOrigin( {}, CredentialCache::IsOriginBlocklisted(true), std::nullopt, url::Origin::Create(GURL(kExampleSite))); - ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite))) + ON_CALL(*password_client(), + IsSavingAndFillingEnabled(url::Origin::Create(GURL(kExampleSite)), _)) .WillByDefault(Return(true)); EXPECT_CALL(filling_source_observer_, @@ -958,7 +959,8 @@ // Simulate saving being disabled (e.g. being in incognito or having password // saving disabled from settings). - ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite))) + ON_CALL(*password_client(), + IsSavingAndFillingEnabled(url::Origin::Create(GURL(kExampleSite)), _)) .WillByDefault(Return(false)); cache()->SaveCredentialsAndBlocklistedForOrigin( @@ -986,7 +988,8 @@ cache()->SaveCredentialsAndBlocklistedForOrigin( {}, CredentialCache::IsOriginBlocklisted(false), std::nullopt, url::Origin::Create(GURL(kExampleSite))); - ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite))) + ON_CALL(*password_client(), + IsSavingAndFillingEnabled(url::Origin::Create(GURL(kExampleSite)), _)) .WillByDefault(Return(true)); EXPECT_CALL(filling_source_observer_, @@ -1009,7 +1012,8 @@ cache()->SaveCredentialsAndBlocklistedForOrigin( {}, CredentialCache::IsOriginBlocklisted(true), std::nullopt, url::Origin::Create(GURL(kExampleSite))); - ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite))) + ON_CALL(*password_client(), + IsSavingAndFillingEnabled(url::Origin::Create(GURL(kExampleSite)), _)) .WillByDefault(Return(true)); EXPECT_CALL(filling_source_observer_, @@ -1038,7 +1042,8 @@ cache()->SaveCredentialsAndBlocklistedForOrigin( {}, CredentialCache::IsOriginBlocklisted(true), std::nullopt, url::Origin::Create(GURL(kExampleSite))); - ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite))) + ON_CALL(*password_client(), + IsSavingAndFillingEnabled(url::Origin::Create(GURL(kExampleSite)), _)) .WillByDefault(Return(true)); EXPECT_CALL(filling_source_observer_, @@ -1063,7 +1068,8 @@ {}, CredentialCache::IsOriginBlocklisted(false), std::nullopt, url::Origin::Create(GURL(kExampleSite))); - ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite))) + ON_CALL(*password_client(), + IsSavingAndFillingEnabled(url::Origin::Create(GURL(kExampleSite)), _)) .WillByDefault(Return(true)); EXPECT_CALL(filling_source_observer_, Run(controller(), IsFillingSourceAvailable(true))); diff --git a/chrome/browser/password_manager/chrome_password_manager_client.cc b/chrome/browser/password_manager/chrome_password_manager_client.cc index 2ae9f70..0433005 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.cc +++ b/chrome/browser/password_manager/chrome_password_manager_client.cc @@ -265,7 +265,8 @@ ChromePasswordManagerClient::~ChromePasswordManagerClient() = default; bool ChromePasswordManagerClient::IsSavingAndFillingEnabled( - const GURL& url) const { + const url::Origin& origin, + base::optional_ref<const GURL> url) const { if (base::CommandLine::ForCurrentProcess()->HasSwitch( switches::kEnableAutomation)) { // Disable the password saving UI for automated tests. It obscures the @@ -277,7 +278,7 @@ return settings_service && settings_service->IsSettingEnabled( PasswordManagerSetting::kOfferToSavePasswords) && - !IsOffTheRecord() && IsFillingEnabled(url::Origin::Create(url)); + !IsOffTheRecord() && IsFillingEnabled(origin, url); } bool ChromePasswordManagerClient::IsFillingEnabled( @@ -2204,7 +2205,7 @@ } void ChromePasswordManagerClient::MaybeShowSavePasswordPrimingPromo( - const GURL& current_url) { + const url::Origin& origin) { // If the user has any stored passwords do not show the promo. Profile* profile = Profile::FromBrowserContext(web_contents()->GetBrowserContext()); @@ -2214,9 +2215,7 @@ return; } - // If the current page is not eligible for password saving, do not show the - // promo. - if (!IsSavingAndFillingEnabled(current_url)) { + if (!IsSavingAndFillingEnabled(origin)) { return; } diff --git a/chrome/browser/password_manager/chrome_password_manager_client.h b/chrome/browser/password_manager/chrome_password_manager_client.h index 816c46cf..89d40cbb 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.h +++ b/chrome/browser/password_manager/chrome_password_manager_client.h @@ -138,7 +138,10 @@ // PasswordManagerClient implementation. using password_manager::PasswordManagerClient::IsFillingEnabled; - bool IsSavingAndFillingEnabled(const GURL& url) const override; + using password_manager::PasswordManagerClient::IsSavingAndFillingEnabled; + bool IsSavingAndFillingEnabled( + const url::Origin& origin, + base::optional_ref<const GURL> url) const override; bool IsFillingEnabled(const url::Origin& origin, base::optional_ref<const GURL> url) const override; bool IsFieldFilledWithOtp(autofill::FormGlobalId form_id, @@ -488,7 +491,7 @@ autofill::password_generation::PasswordGenerationType type, password_manager::ContentPasswordManagerDriver* driver, const autofill::password_generation::PasswordGenerationUIData& ui_data); - void MaybeShowSavePasswordPrimingPromo(const GURL& current_url) override; + void MaybeShowSavePasswordPrimingPromo(const url::Origin& origin) override; #endif // !BUILDFLAG(IS_ANDROID) gfx::RectF TransformToRootCoordinates( diff --git a/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc b/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc index 4562222..fe3f689 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc +++ b/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc @@ -606,7 +606,7 @@ IsSettingEnabled(PasswordManagerSetting::kOfferToSavePasswords)) .WillByDefault(Return(true)); const GURL kUrlOn("https://accounts.google.com"); - EXPECT_TRUE(client->IsSavingAndFillingEnabled(kUrlOn)); + EXPECT_TRUE(client->IsSavingAndFillingEnabled(url::Origin::Create(kUrlOn))); }
Regression Test / PoC
diff --git a/chrome/browser/keyboard_accessory/android/password_accessory_controller_impl_unittest.cc b/chrome/browser/keyboard_accessory/android/password_accessory_controller_impl_unittest.cc
index a6b33f2..11e8113 100644
--- a/chrome/browser/keyboard_accessory/android/password_accessory_controller_impl_unittest.cc
+++ b/chrome/browser/keyboard_accessory/android/password_accessory_controller_impl_unittest.cc
@@ -168,7 +168,7 @@
MOCK_METHOD(bool,
IsSavingAndFillingEnabled,
- (const GURL&),
+ (const url::Origin&, base::optional_ref<const GURL>),
(const, override));
MOCK_METHOD(std::unique_ptr<device_reauth::DeviceAuthenticator>,
@@ -934,7 +934,8 @@
cache()->SaveCredentialsAndBlocklistedForOrigin(
{}, CredentialCache::IsOriginBlocklisted(true), std::nullopt,
url::Origin::Create(GURL(kExampleSite)));
- ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite)))
+ ON_CALL(*password_client(),
+ IsSavingAndFillingEnabled(url::Origin::Create(GURL(kExampleSite)), _))
.WillByDefault(Return(true));
EXPECT_CALL(filling_source_observer_,
@@ -958,7 +959,8 @@
// Simulate saving being disabled (e.g. being in incognito or having password
// saving disabled from settings).
- ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite)))
+ ON_CALL(*password_client(),
+ IsSavingAndFillingEnabled(url::Origin::Create(GURL(kExampleSite)), _))
.WillByDefault(Return(false));
cache()->SaveCredentialsAndBlocklistedForOrigin(
@@ -986,7 +988,8 @@
cache()->SaveCredentialsAndBlocklistedForOrigin(
{}, CredentialCache::IsOriginBlocklisted(false), std::nullopt,
url::Origin::Create(GURL(kExampleSite)));
- ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite)))
+ ON_CALL(*password_client(),
+ IsSavingAndFillingEnabled(url::Origin::Create(GURL(kExampleSite)), _))
.WillByDefault(Return(true));
EXPECT_CALL(filling_source_observer_,
@@ -1009,7 +1012,8 @@
cache()->SaveCredentialsAndBlocklistedForOrigin(
{}, CredentialCache::IsOriginBlocklisted(true), std::nullopt,
url::Origin::Create(GURL(kExampleSite)));
- ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite)))
+ ON_CALL(*password_client(),
+ IsSavingAndFillingEnabled(url::Origin::Create(GURL(kExampleSite)), _))
.WillByDefault(Return(true));
EXPECT_CALL(filling_source_observer_,
@@ -1038,7 +1042,8 @@
cache()->SaveCredentialsAndBlocklistedForOrigin(
{}, CredentialCache::IsOriginBlocklisted(true), std::nullopt,
url::Origin::Create(GURL(kExampleSite)));
- ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite)))
+ ON_CALL(*password_client(),
+ IsSavingAndFillingEnabled(url::Origin::Create(GURL(kExampleSite)), _))
.WillByDefault(Return(true));
EXPECT_CALL(filling_source_observer_,
@@ -1063,7 +1068,8 @@
{}, CredentialCache::IsOriginBlocklisted(false), std::nullopt,
url::Origin::Create(GURL(kExampleSite)));
- ON_CALL(*password_client(), IsSavingAndFillingEnabled(GURL(kExampleSite)))
+ ON_CALL(*password_client(),
+ IsSavingAndFillingEnabled(url::Origin::Create(GURL(kExampleSite)), _))
.WillByDefault(Return(true));
EXPECT_CALL(filling_source_observer_,
Run(controller(), IsFillingSourceAvailable(true)));
diff --git a/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc b/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
index 4562222..fe3f689 100644
--- a/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
+++ b/chrome/browser/password_manager/chrome_password_manager_client_unittest.cc
@@ -606,7 +606,7 @@
IsSettingEnabled(PasswordManagerSetting::kOfferToSavePasswords))
.WillByDefault(Return(true));
const GURL kUrlOn("https://accounts.google.com");
- EXPECT_TRUE(client->IsSavingAndFillingEnabled(kUrlOn));
+ EXPECT_TRUE(client->IsSavingAndFillingEnabled(url::Origin::Create(kUrlOn)));
}
TEST_F(ChromePasswordManagerClientTest,
@@ -617,7 +617,7 @@
IsSettingEnabled(PasswordManagerSetting::kOfferToSavePasswords))
.WillOnce(Return(false));
const GURL kUrlOn("https://accounts.google.com");
- EXPECT_FALSE(client->IsSavingAndFillingEnabled(kUrlOn));
+ EXPECT_FALSE(client->IsSavingAndFillingEnabled(url::Origin::Create(kUrlOn)));
}
TEST_F(ChromePasswordManagerClientTest, SavingAndFillingEnabledConditionsTest) {
@@ -633,7 +633,7 @@
EXPECT_CALL(*client, GetMainFrameCertStatus())
.WillRepeatedly(Return(net::CERT_STATUS_AUTHORITY_INVALID));
const GURL kUrlOn("https://accounts.google.com");
- EXPECT_FALSE(client->IsSavingAndFillingEnabled(kUrlOn));
+ EXPECT_FALSE(client->IsSavingAndFillingEnabled(url::Origin::Create(kUrlOn)));
EXPECT_FALSE(client->IsFillingEnabled(url::Origin::Create(kUrlOn)));
// Disable password saving.
@@ -643,13 +643,13 @@
// Functionality disabled if there are SSL errors and the manager itself is
// disabled.
- EXPECT_FALSE(client->IsSavingAndFillingEnabled(kUrlOn));
+ EXPECT_FALSE(client->IsSavingAndFillingEnabled(url::Origin::Create(kUrlOn)));
EXPECT_FALSE(client->IsFillingEnabled(url::Origin::Create(kUrlOn)));
// Saving disabled if there are no SSL errors, but the manager itself is
// disabled.
EXPECT_CALL(*client, GetMainFrameCertStatus()).WillRepeatedly(Return(0));
- EXPECT_FALSE(client->IsSavingAndFillingEnabled(kUrlOn));
+ EXPECT_FALSE(client->IsSavingAndFillingEnabled(url::Origin::Create(kUrlOn)));
EXPECT_TRUE(client->IsFillingEnabled(url::Origin::Create(kUrlOn)));
// Enable password saving.
@@ -660,7 +660,7 @@
// Functionality enabled if there are no SSL errors and the manager is
// enabled.
EXPECT_CALL(*client, GetMainFrameCertStatus()).WillRepeatedly(Return(0));
- EXPECT_TRUE(client->IsSavingAndFillingEnabled(kUrlOn));
+ EXPECT_TRUE(client->IsSavingAndFillingEnabled(url::Origin::Create(kUrlOn)));
EXPECT_TRUE(client->IsFillingEnabled(url::Origin::Create(kUrlOn)));
}
@@ -681,7 +681,7 @@
// Saving disabled in Incognito mode.
const GURL kUrlOn("https://accounts.google.com");
- EXPECT_FALSE(client->IsSavingAndFillingEnabled(kUrlOn));
+ EXPECT_FALSE(client->IsSavingAndFillingEnabled(url::Origin::Create(kUrlOn)));
EXPECT_TRUE(client->IsFillingEnabled(url::Origin::Create(kUrlOn)));
// In guest mode saving, filling and manual filling are disabled.
@@ -690,7 +690,7 @@
->GetPrimaryOTRProfile(/*create_if_needed=*/true)
->AsTestingProfile()
->SetGuestSession(true);
- EXPECT_FALSE(client->IsSavingAndFillingEnabled(kUrlOn));
+ EXPECT_FALSE(client->IsSavingAndFillingEnabled(url::Origin::Create(kUrlOn)));
EXPECT_FALSE(client->IsFillingEnabled(url::Origin::Create(kUrlOn)));
}
@@ -813,8 +813,8 @@
.empty());
// Expect the password manager to be disallowed for the URL
// and thus saving passwords should be disallowed.
- EXPECT_FALSE(
- GetClient()->IsSavingAndFillingEnabled(GURL("https://example.com")));
+ EXPECT_FALSE(GetClient()->IsSavingAndFillingEnabled(
+ url::Origin::Create(GURL("https://example.com"))));
// Clear the blocklist pref.
profile()->GetTestingPrefService()->ClearPref(
password_manager::prefs::kPasswordManagerBlocklist);
@@ -824,8 +824,8 @@
->GetList(password_manager::prefs::kPasswordManagerBlocklist)
.empty());
// Password manager and saving passwords should be allowed again
- EXPECT_TRUE(
- GetClient()->IsSavingAndFillingEnabled(GURL("https://example.com")));
+ EXPECT_TRUE(GetClient()->IsSavingAndFillingEnabled(
+ url::Origin::Create(GURL("https://example.com"))));
}
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) ||
// BUILDFLAG(IS_CHROMEOS)
@@ -1092,7 +1092,8 @@
IsSettingEnabled(PasswordManagerSetting::kOfferToSavePasswords))
.WillByDefault(Return(true));
const GURL kUrlOn("https://accounts.google.com");
- EXPECT_NE(client->IsSavingAndFillingEnabled(kUrlOn), GetParam());
+ EXPECT_NE(client->IsSavingAndFillingEnabled(url::Origin::Create(kUrlOn)),
+ GetParam());
}
// Check that password manager is disabled on about:blank pages.
@@ -1101,7 +1102,8 @@
const GURL kUrl(url::kAboutBlankURL);
NavigateAndCommit(kUrl);
EXPECT_TRUE(GetClient()->GetLastCommittedOrigin().opaque());
- EXPECT_FALSE(GetClient()->IsSavingAndFillingEnabled(kUrl));
+ EXPECT_FALSE(
+ GetClient()->IsSavingAndFillingEnabled(url::Origin::Create(kUrl)));
EXPECT_FALSE(GetClient()->IsFillingEnabled(url::Origin::Create(kUrl)));
}
@@ -1152,7 +1154,7 @@
IsFillingAndSavingOnGooglePasswordPage) {
PasswordManagerClient* client = GetClient();
EXPECT_FALSE(client->IsSavingAndFillingEnabled(
- GURL("https://passwords.google.com/path?query=1")));
+ url::Origin::Create(GURL("https://passwords.google.com/path?query=1"))));
EXPECT_FALSE(client->IsFillingEnabled(
url::Origin::Create(GURL("https://passwords.google.com/path?query=1"))));
}
@@ -1391,7 +1393,7 @@
.WillByDefault(Return(true));
ASSERT_FALSE(it == std::end(kSchemeTestCases));
EXPECT_EQ(it->password_manager_works,
- GetClient()->IsSavingAndFillingEnabled(url));
+ GetClient()->IsSavingAndFillingEnabled(url::Origin::Create(url)));
EXPECT_EQ(it->password_manager_works,
GetClient()->IsFillingEnabled(url::Origin::Create(url)));
}
@@ -1583,7 +1585,7 @@
ON_CALL(*client, GetMainFrameCertStatus()).WillByDefault(Return(0));
// Saving is disabled when the page has a delayed SafeBrowsing warning.
const GURL kUrlOn("https://accounts.google.com");
- EXPECT_FALSE(client->IsSavingAndFillingEnabled(kUrlOn));
+ EXPECT_FALSE(client->IsSavingAndFillingEnabled(url::Origin::Create(kUrlOn)));
EXPECT_FALSE(client->IsFillingEnabled(url::Origin::Create(kUrlOn)));
}
#endif
diff --git a/components/password_manager/core/browser/credential_manager_impl_unittest.cc b/components/password_manager/core/browser/credential_manager_impl_unittest.cc
index a7f0b0a..047d25f 100644
--- a/components/password_manager/core/browser/credential_manager_impl_unittest.cc
+++ b/components/password_manager/core/browser/credential_manager_impl_unittest.cc
@@ -86,7 +86,7 @@
public:
MOCK_METHOD(bool,
IsSavingAndFillingEnabled,
- (const GURL&),
+ (const url::Origin&, base::optional_ref<const GURL>),
(const, override));
MOCK_METHOD(bool,
IsFillingEnabled,
@@ -822,7 +822,8 @@
TEST_P(CredentialManagerImplTest,
CredentialManagerSignInWithSavingDisabledForCurrentPage) {
auto info = PasswordFormToCredentialInfo(form_);
- EXPECT_CALL(*client_, IsSavingAndFillingEnabled(form_.url))
+ EXPECT_CALL(*client_,
+ IsSavingAndFillingEnabled(url::Origin::Create(form_.url), _))
.WillRepeatedly(Return(false));
EXPECT_CALL(*client_, PromptUserToSaveOrUpdatePassword).Times(0);
EXPECT_CALL(*client_, NotifyStorePasswordCalled).Times(0);
diff --git a/components/password_manager/core/browser/http_auth_manager_unittest.cc b/components/password_manager/core/browser/http_auth_manager_unittest.cc
index 9bce55bf..b119eb2 100644
--- a/components/password_manager/core/browser/http_auth_manager_unittest.cc
+++ b/components/password_manager/core/browser/http_auth_manager_unittest.cc
@@ -60,7 +60,7 @@
public:
MOCK_METHOD(bool,
IsSavingAndFillingEnabled,
- (const GURL&),
+ (const url::Origin&, base::optional_ref<const GURL>),
(const, override));
MOCK_METHOD(bool,
IsFillingEnabled,
@@ -520,7 +520,7 @@
SCOPED_TRACE(testing::Message("filling_and_saving_enabled=")
<< filling_and_saving_enabled);
- EXPECT_CALL(client_, IsSavingAndFillingEnabled(_))
+ EXPECT_CALL(client_, IsSavingAndFillingEnabled(_, _))
.WillRepeatedly(Return(filling_and_saving_enabled));
EXPECT_CALL(client_, IsFillingEnabled)
.WillRepeatedly(Return(filling_and_saving_enabled));
@@ -623,7 +623,7 @@
}
TEST_P(HttpAuthManagerTest, NavigationWithoutSubmission) {
- EXPECT_CALL(client_, IsSavingAndFillingEnabled(_))
+ EXPECT_CALL(client_, IsSavingAndFillingEnabled(_, _))
.WillRepeatedly(Return(true));
EXPECT_CALL(client_, IsFillingEnabled).WillRepeatedly(Return(true));
PasswordForm observed_form;
diff --git a/components/password_manager/core/browser/password_form_filling_unittest.cc b/components/password_manager/core/browser/password_form_filling_unittest.cc
index 64137700..3879b94 100644
--- a/components/password_manager/core/browser/password_form_filling_unittest.cc
+++ b/components/password_manager/core/browser/password_form_filling_unittest.cc
@@ -77,7 +77,7 @@
(override));
MOCK_METHOD(bool,
IsSavingAndFillingEnabled,
- (const GURL&),
+ (const url::Origin&, base::optional_ref<const GURL>),
(const, override));
MOCK_METHOD(bool, IsCommittedMainFrameSecure, (), (const, override));
MOCK_METHOD(MockWebAuthnCredentialsDelegate*,
diff --git a/components/password_manager/core/browser/password_form_manager_unittest.cc b/components/password_manager/core/browser/password_form_manager_unittest.cc
index 67fc122..bdd9e96 100644
--- a/components/password_manager/core/browser/password_form_manager_unittest.cc
+++ b/components/password_manager/core/browser/password_form_manager_unittest.cc
@@ -220,7 +220,7 @@
public:
MOCK_METHOD(bool,
IsSavingAndFillingEnabled,
- (const GURL&),
+ (const url::Origin&, base::optional_ref<const GURL>),
(const, override));
MOCK_METHOD(bool,
IsFieldFilledWithOtp,
@@ -948,7 +948,9 @@
}
TEST_P(PasswordFormManagerTest, TestSaveFormAllowedNegative) {
... (truncated)
Original Bug Report
Potential cross-origin credential leak via ProcessAutofillPredictions
Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.
Overview: A potential vulnerability exists where a same-site sandboxed iframe with an opaque origin can steal parent origin credentials. This occurs because the browser-initiated ProcessAutofillPredictions path creates a PasswordFormManager without validating if the frame has an opaque origin, allowing the sandboxed frame to fetch the parent’s autofill data. An attacker can exploit this by dynamically changing a single-username field to a password field during an autofill interaction to exfiltrate the password.
Affected files:
components/password_manager/core/browser/password_manager.ccchrome/browser/password_manager/chrome_password_manager_client.cccomponents/password_manager/core/browser/password_autofill_manager.cccomponents/password_manager/content/browser/content_password_manager_driver.cc
Estimated timestamp from git blame: Unknown (Google3 checkout)
Summary
A potential vulnerability in the Password Manager allows a same-site sandboxed iframe to obtain plaintext saved passwords belonging to its parent origin. This bypasses the security boundaries intended by the sandbox attribute (specifically when allow-same-origin is omitted, resulting in an opaque origin).
The root cause is that the browser-initiated prediction processing path (PasswordManager::ProcessAutofillPredictions) creates a PasswordFormManager for drivers associated with sandboxed iframes without validating if the frame’s committed origin is opaque. This results in the parent origin’s credentials being fetched and stored in the browser-side PasswordAutofillManager::fill_data_. When a user interacts with a single-username field and selects an autofill suggestion, an attacker can dynamically change the field type to password to force the renderer to fill the plaintext password into the DOM, where it can be read by malicious JavaScript.
Potential Attack Chain
Note: These are suggested steps based on static code analysis; a working Proof of Concept has not been run.
- Attacker Setup: An attacker embeds an iframe on the victim’s site (e.g., via user-generated content) with
sandbox="allow-scripts"(omittingallow-same-origin). This forces the iframe into an opaque origin. - Form Injection: The attacker injects a simple HTML form containing a single text input field (e.g.,
<input type="text" id="username">) into the sandboxed iframe. - Prediction Trigger: The browser receives Autofill server predictions evaluating the form as a
SINGLE_USERNAMEfield. - Bypassing Validation:
PasswordManager::ProcessAutofillPredictionsis called and executesCreateFormManager(driver, form). WhileCreateFormManagerchecksclient_->IsFillingEnabled(form.url()), it fails to check if the frame has an opaque origin (unlike renderer-initiated paths which useHasValidURL()). - Credential Fetching: A
PasswordFormManageris created, which fetches the victim’s saved credentials for the parent origin and stores the full plaintext password in the browser’sPasswordAutofillManager::fill_data_. - User Interaction & Type Confusion: The user clicks the text input field, triggering the Autofill suggestion popup. While the popup is open, the attacker’s JavaScript dynamically alters the
typeattribute of the input element fromtexttopassword. - Preserved Identity: Changing the
typeattribute preserves the element’sblink::WebNode::GetDomNodeId(), meaning itsFieldRendererIdandFieldRefdictionary key remain unchanged. - Suggestion Selection & IPC: The user selects the suggestion. The browser retrieves the plaintext password from
fill_data_and sends it via theFillPasswordSuggestionMojo IPC to the renderer. - Renderer Filling: In the renderer,
PasswordAutofillAgent::FillPasswordSuggestioncallsFindPasswordInfoForElement. Because the attacker script changed the type topassword, it successfully looks up the element inweb_input_to_password_info_using the preservedFieldRef. The validation passes, andblink::WebInputElement::SetAutofillValueinjects the plaintext password into the DOM. - Exfiltration: The attacker’s script reads the
.valueproperty of the input element, stealing the password.
Affected Code Locations
components/password_manager/core/browser/password_manager.cc:ProcessAutofillPredictionscallsCreateFormManagerwithout an opaque origin check.components/password_manager/core/browser/password_manager.cc:CreateFormManagerrelies onIsFillingEnabled, which doesn’t check the origin’s opaque status.
Suggested Fix
Add an origin validation check to PasswordManager::ProcessAutofillPredictions or PasswordManager::CreateFormManager to ensure that a PasswordFormManager is not created for frames with an opaque origin. This could involve checking driver->HasValidURL() or explicitly verifying that the frame’s GetLastCommittedOrigin() is not opaque before proceeding with form manager creation.
Evaluated with Chrome root at commit: 65b3256311f3ab6fb9870eaa522de7e6dd2663bb
Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:
- If you are familiar with the severity guidelines, you may adjust the severity.
- If this is a false positive, and there’s no work to be done, please close as WAI.
- If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.
Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.