diff --git a/chrome/browser/password_manager/password_change_browsertest.cc b/chrome/browser/password_manager/password_change_browsertest.cc index 73efe78008113..41ed3d6c4dcd5 100644 --- a/chrome/browser/password_manager/password_change_browsertest.cc +++ b/chrome/browser/password_manager/password_change_browsertest.cc @@ -57,6 +57,7 @@ #include "components/optimization_guide/core/optimization_guide_features.h" #include "components/optimization_guide/core/optimization_guide_prefs.h" #include "components/optimization_guide/core/optimization_guide_proto_util.h" +#include "components/optimization_guide/proto/features/password_change_submission.pb.h" #include "components/optimization_guide/proto/model_quality_service.pb.h" #include "components/password_manager/core/browser/features/password_features.h" #include "components/password_manager/core/browser/password_form.h" @@ -551,6 +552,145 @@ IN_PROC_BROWSER_TEST_F(PasswordChangeBrowserTest, NewPasswordIsSaved) { FinalModelStatus::FINAL_MODEL_STATUS_SUCCESS); } +// POC: PasswordChangeSubmissionVerifier omits include_same_site_only, so +// content from a cross-site iframe (e.g., a third-party ad/widget) on the +// post-submission page is captured and shipped to the verification model. +// An adversary controlling that iframe can place "success" text that the +// model may interpret as SUCCESSFUL_OUTCOME, causing Chrome to overwrite the +// stored credential and show trusted "password changed" UI even though the +// first-party site rejected the change. +IN_PROC_BROWSER_TEST_F(PasswordChangeBrowserTest, + CrossSiteIframeContentReachesVerificationModel_POC) { + SetPrivacyNoticeAcceptedPref(); + SetChangePasswordUrl("/password/poc_update_form_with_xsite_iframe.html"); + + password_change_service()->OfferPasswordChangeUi( + CreatePasswordForm(WebContents()->GetLastCommittedURL(), u"test", + u"pa$$word"), + WebContents()); + PasswordChangeDelegate* delegate = + password_change_service()->GetPasswordChangeDelegate(WebContents()); + delegate->StartPasswordChangeFlow(); + MockLoginOutcome(LoginCheckResult::kLoggedIn); + + static constexpr char kAttackerMarker[] = + "ATTACKER_INJECTED_SUCCESS_MARKER"; + + // ---- SUBMIT_FORM_STEP: capture the model request, then click submit ---- + std::string submit_step_serialized_apc; + { + SetWebContents( + static_cast(delegate)->executor()); + base::RunLoop run_loop; + EXPECT_CALL(*mock_optimization_guide_keyed_service(), + ExecuteModel(optimization_guide::ModelBasedCapabilityKey:: + kPasswordChangeSubmission, + _, _, _)) + .WillOnce( + DoAll(testing::Invoke(&run_loop, &base::RunLoop::Quit), + testing::WithArgs<1, 3>( + [&](const google::protobuf::MessageLite& msg, + auto callback) { + optimization_guide::proto::PasswordChangeRequest req; + CHECK(req.ParseFromString(msg.SerializeAsString())); + submit_step_serialized_apc = + req.page_context() + .annotated_page_content() + .SerializeAsString(); + LOG(ERROR) + << "[POC] step=" << req.step() + << " (SUBMIT_FORM_STEP) APC bytes=" + << submit_step_serialized_apc.size() + << " contains-attacker-text=" + << (submit_step_serialized_apc.find( + kAttackerMarker) != std::string::npos); + optimization_guide::proto::PasswordChangeResponse + response; + response.mutable_submit_form_data() + ->set_dom_node_id_to_click(GetDomNodeId( + "chg_submit_wo_username_button")); + std::move(callback).Run( + optimization_guide:: + OptimizationGuideModelExecutionResult( + optimization_guide::AnyWrapProto(response), + /*execution_info=*/nullptr), + /*log_entry=*/nullptr); + }))); + run_loop.Run(); + SetWebContents(browser()->tab_strip_model()->GetWebContentsAt(0)); + } + + // ---- VERIFY_SUBMISSION_STEP: capture the model request ---- + std::string verify_step_serialized_apc; + { + base::RunLoop run_loop; + EXPECT_CALL(*mock_optimization_guide_keyed_service(), + ExecuteModel(optimization_guide::ModelBasedCapabilityKey:: + kPasswordChangeSubmission, + _, _, _)) + .WillOnce( + DoAll(testing::Invoke(&run_loop, &base::RunLoop::Quit), + testing::WithArgs<1, 3>( + [&](const google::protobuf::MessageLite& msg, + auto callback) { + optimization_guide::proto::PasswordChangeRequest req; + CHECK(req.ParseFromString(msg.SerializeAsString())); + verify_step_serialized_apc = + req.page_context() + .annotated_page_content() + .SerializeAsString(); + LOG(ERROR) + << "[POC] step=" << req.step() + << " (VERIFY_SUBMISSION_STEP) APC bytes=" + << verify_step_serialized_apc.size() + << " contains-attacker-text=" + << (verify_step_serialized_apc.find( + kAttackerMarker) != std::string::npos); + // Simulate the model interpreting attacker iframe + // text as a successful change. + optimization_guide::proto::PasswordChangeResponse + response; + response.mutable_outcome_data() + ->set_submission_outcome( + PasswordChangeOutcome:: + PasswordChangeSubmissionData_PasswordChangeOutcome_SUCCESSFUL_OUTCOME); + std::move(callback).Run( + optimization_guide:: + OptimizationGuideModelExecutionResult( + optimization_guide::AnyWrapProto(response), + /*execution_info=*/nullptr), + /*log_entry=*/nullptr); + }))); + run_loop.Run(); + } + + // The submit-form step sets include_same_site_only=true, so the cross-site + // iframe is redacted: attacker text must NOT reach that model request. + EXPECT_EQ(submit_step_serialized_apc.find(kAttackerMarker), + std::string::npos) + << "[POC] Submit step unexpectedly contains cross-site iframe content"; + + // The verify-submission step omits include_same_site_only, so the + // cross-site iframe content IS shipped to the model. This is the bug. + EXPECT_NE(verify_step_serialized_apc.find(kAttackerMarker), + std::string::npos) + << "[POC] BUG NOT REPRODUCED: cross-site iframe content was redacted " + "at the verify step"; + + // Because the (mocked) model returned SUCCESSFUL_OUTCOME based on the + // attacker-influenced page content, Chrome now believes the change + // succeeded, overwrites the stored credential, and will show trusted + // native success UI -- even though the first party rendered an error. + EXPECT_EQ(PasswordChangeDelegate::State::kPasswordSuccessfullyChanged, + delegate->GetCurrentState()); + WaitForPasswordStore(); + CheckThatCredentialsStored( + /*username=*/"test", + base::UTF16ToUTF8(static_cast(delegate) + ->generated_password()), + "pa$$word", password_manager::PasswordForm::Type::kChangeSubmission); +} + IN_PROC_BROWSER_TEST_F(PasswordChangeBrowserTest, OldPasswordIsUpdated) { SetPrivacyNoticeAcceptedPref();