diff --git a/chrome/browser/password_manager/password_change/change_password_form_waiter_unittest.cc b/chrome/browser/password_manager/password_change/change_password_form_waiter_unittest.cc index 1a7a4fab710ad..f2f5d9fe5d16b 100644 --- a/chrome/browser/password_manager/password_change/change_password_form_waiter_unittest.cc +++ b/chrome/browser/password_manager/password_change/change_password_form_waiter_unittest.cc @@ -132,8 +132,15 @@ class ChangePasswordFormWaiterTest : public ChromeRenderViewHostTestHarness { std::unique_ptr CreateFormManager( const autofill::FormData& form_data) { + return CreateFormManagerForDriver(form_data, driver()); + } + + std::unique_ptr + CreateFormManagerForDriver( + const autofill::FormData& form_data, + password_manager::StubPasswordManagerDriver& form_driver) { auto form_manager = std::make_unique( - client(), driver().AsWeakPtr(), form_data, &form_fetcher(), + client(), form_driver.AsWeakPtr(), form_data, &form_fetcher(), std::make_unique(client()), /*metrics_recorder=*/nullptr); // Force form parsing, otherwise there will be no parsed observed form. @@ -840,3 +847,103 @@ TEST_F(ChangePasswordFormWaiterTest, FeatureEnabled_ModelNotAvailable_Timeout) { task_environment()->FastForwardBy( ChangePasswordFormWaiter::kChangePasswordFormWaitingTimeout); } + +// SECURITY: GetCorrespondingFormManager() re-resolves the PasswordFormManager +// after the async CheckViewAreaVisible round-trip by matching ONLY on +// new_password_element_renderer_id. FieldRendererId is documented as "unique +// only within one frame" (components/autofill/core/common/unique_ids.h). The +// PasswordFormCache is shared across all frames in a WebContents, so a cross- +// origin iframe whose new-password field has a numerically colliding renderer +// id can be returned in place of the main-frame manager that actually passed +// the visibility check. This test demonstrates that the callback fires with +// the iframe's PasswordFormManager (whose driver belongs to the attacker +// frame) even though the visibility check ran against the main-frame driver. +TEST_F(ChangePasswordFormWaiterTest, + CrossFrameRendererIdCollisionReturnsWrongManager) { + // Second driver simulating a cross-origin iframe in the same WebContents. + testing::NiceMock attacker_iframe_driver; + + // Both forms share the SAME FieldRendererId for the new-password field. In + // production this happens when the iframe (in its own site-isolated renderer + // process) mints DOMNodeIds that numerically collide with the main frame's. + static constexpr autofill::FieldRendererId kCollidingNewPasswordId(2); + + auto make_change_pwd_form = [](const char* url) { + std::vector fields; + fields.push_back(CreateTestFormField( + /*label=*/"Password:", /*name=*/"password", + /*value=*/"", autofill::FormControlType::kInputPassword)); + fields.back().set_renderer_id(autofill::FieldRendererId(1)); + fields.push_back(CreateTestFormField( + /*label=*/"New password:", /*name=*/"new_password_1", + /*value=*/"", autofill::FormControlType::kInputPassword)); + fields.back().set_renderer_id(kCollidingNewPasswordId); + fields.push_back(CreateTestFormField( + /*label=*/"Password confirmation:", /*name=*/"new_password_2", + /*value=*/"", autofill::FormControlType::kInputPassword)); + fields.back().set_renderer_id(autofill::FieldRendererId(3)); + autofill::FormData form; + form.set_url(GURL(url)); + form.set_fields(std::move(fields)); + return form; + }; + + // The cache contains [attacker_iframe_manager, main_frame_manager] — the + // attacker arranged its form to be parsed/inserted first. + std::vector> + form_managers; + form_managers.push_back(CreateFormManagerForDriver( + make_change_pwd_form("https://attacker.example"), + attacker_iframe_driver)); + form_managers.push_back(CreateFormManagerForDriver( + make_change_pwd_form("https://victim.example"), driver())); + + password_manager::PasswordFormManager* const attacker_manager = + form_managers[0].get(); + password_manager::PasswordFormManager* const main_frame_manager = + form_managers[1].get(); + ASSERT_NE(attacker_manager->GetDriver().get(), + main_frame_manager->GetDriver().get()); + ASSERT_EQ( + attacker_manager->GetParsedObservedForm() + ->new_password_element_renderer_id, + main_frame_manager->GetParsedObservedForm() + ->new_password_element_renderer_id); + + base::MockOnceCallback + completion_callback; + auto waiter = ChangePasswordFormWaiter::Builder(web_contents(), client(), + completion_callback.Get()) + .Build(); + + EXPECT_CALL(cache(), GetFormManagers) + .WillRepeatedly(testing::Return(base::span(form_managers))); + + // The attacker iframe's driver should never be consulted for visibility: + // the waiter is processing the *main-frame* form. + EXPECT_CALL(attacker_iframe_driver, CheckViewAreaVisible).Times(0); + // The main-frame driver's visibility check passes. + EXPECT_CALL(driver(), + CheckViewAreaVisible(kCollidingNewPasswordId, testing::_)) + .WillOnce(base::test::RunOnceCallback<1>(true)); + + // BUG: The completion callback fires with the attacker iframe's manager — + // its GetDriver() points at the cross-origin frame, which becomes the fill + // target for the user's plaintext password in + // ChangePasswordFormFillingSubmissionHelper::TriggerFilling(). + // A correct implementation would key the re-lookup on (driver, renderer_id) + // and return `main_frame_manager` here. + password_manager::PasswordFormManager* delivered_manager = nullptr; + EXPECT_CALL(completion_callback, Run(testing::_)) + .WillOnce(testing::SaveArg<0>(&delivered_manager)); + static_cast(waiter.get()) + ->OnPasswordFormParsed(main_frame_manager); + + ASSERT_NE(delivered_manager, nullptr); + // VULNERABILITY DEMONSTRATED: the wrong frame's manager was selected. + EXPECT_EQ(delivered_manager, attacker_manager) + << "Expected lookup-by-renderer-id-only to return the attacker iframe's " + "manager (the bug). If this fails the bug may already be fixed."; + EXPECT_EQ(delivered_manager->GetDriver().get(), &attacker_iframe_driver); + EXPECT_NE(delivered_manager, main_frame_manager); +}