From 439393b368e0d9f838b0ed87e507bda0c8afbd99 Mon Sep 17 00:00:00 2001
From: Frank Liberato <liberato@chromium.org>
Date: Tue, 01 Sep 2026 13:28:35 -0700
Subject: [PATCH] Fix exit instruction corruption from member shadowing in ExclusiveAccessBubbleAndroid

ExclusiveAccessBubbleAndroid declared a private
ExclusiveAccessBubbleParams params_ member that shadowed the protected
params_ member in the base ExclusiveAccessBubble class. Because
ExclusiveAccessBubble::params() is non-virtual and returns the base
class member, callers invoking params() received the stale,
construction-time parameters rather than the live updated state.

In ExclusiveAccessContextAndroid::OnExclusiveAccessUserInput(), after 10
user interactions the snooze reset mechanism reads
exclusive_access_bubble_->params() to forcibly re-show the security
notice. Due to the shadowing, this read returned the initial params
(e.g., plain fullscreen) instead of updated params (e.g., keyboard
lock). This corrupted the bubble parameters and caused the bridge to
display "press Esc" instead of "press and hold Esc", aiding fullscreen
trapping attacks.

This CL removes the shadowing params_ declaration in
ExclusiveAccessBubbleAndroid so that it accesses and mutates the
inherited base member directly, consistent with
ExclusiveAccessBubbleViews. Unit tests are added to verify live
parameter updates and snooze reset preservation.

TAG=agy
CONV=b9589152-0c17-4890-9676-104a98b7194d

Bug: 553276352
Change-Id: Id99923f0398ca8dd28399af1410285fece195b8c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8335196
Auto-Submit: Frank Liberato <liberato@chromium.org>
Commit-Queue: Frank Liberato <liberato@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Commit-Queue: Peter Conn <peconn@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1690059}
---
diff --git a/chrome/browser/ui/android/exclusive_access/exclusive_access_bubble_android.h b/chrome/browser/ui/android/exclusive_access/exclusive_access_bubble_android.h
index 8c64bf9..8934002b 100644
--- a/chrome/browser/ui/android/exclusive_access/exclusive_access_bubble_android.h
+++ b/chrome/browser/ui/android/exclusive_access/exclusive_access_bubble_android.h
@@ -68,7 +68,6 @@
   std::u16string GetBubbleText(ExclusiveAccessBubbleType bubble_type,
                                bool keyboard_connected) const;
 
-  ExclusiveAccessBubbleParams params_;
   bool notify_overridden_;
   bool was_shown_ = false;
   ExclusiveAccessBubbleHideCallback first_hide_callback_;
diff --git a/chrome/browser/ui/android/exclusive_access/exclusive_access_bubble_android_unittest.cc b/chrome/browser/ui/android/exclusive_access/exclusive_access_bubble_android_unittest.cc
index 976f848..2be564f 100644
--- a/chrome/browser/ui/android/exclusive_access/exclusive_access_bubble_android_unittest.cc
+++ b/chrome/browser/ui/android/exclusive_access/exclusive_access_bubble_android_unittest.cc
@@ -14,6 +14,10 @@
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/strings/grit/ui_strings.h"
+#include "url/gurl.h"
+#include "url/origin.h"
 
 using testing::_;
 using testing::Return;
@@ -152,4 +156,108 @@
   testing::Mock::VerifyAndClearExpectations(mock_bridge_ptr);
 }
 
+TEST_F(ExclusiveAccessBubbleAndroidTest,
+       ParamsAccessorReturnsLiveUpdatedParams) {
+  ExclusiveAccessBubbleParams initial_params;
+  initial_params.type =
+      EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_EXIT_INSTRUCTION;
+
+  auto mock_bridge = std::make_unique<MockBridge>();
+  auto* mock_bridge_ptr = mock_bridge.get();
+
+  EXPECT_CALL(*mock_bridge_ptr, IsVisible()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_bridge_ptr, IsKeyboardConnected()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_bridge_ptr, Update(_)).Times(1);
+  EXPECT_CALL(*mock_bridge_ptr, Show()).Times(1);
+
+  ExclusiveAccessBubbleAndroid bubble(initial_params, base::DoNothing(),
+                                      std::move(mock_bridge));
+  testing::Mock::VerifyAndClearExpectations(mock_bridge_ptr);
+
+  EXPECT_EQ(bubble.params().type,
+            EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_EXIT_INSTRUCTION);
+
+  // Dynamically update the bubble params (e.g. keyboard lock acquired).
+  ExclusiveAccessBubbleParams update_params;
+  update_params.type =
+      EXCLUSIVE_ACCESS_BUBBLE_TYPE_KEYBOARD_LOCK_EXIT_INSTRUCTION;
+  update_params.origin = url::Origin::Create(GURL("https://example.com"));
+
+  EXPECT_CALL(*mock_bridge_ptr, IsVisible()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_bridge_ptr, IsKeyboardConnected()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_bridge_ptr, Update(_)).Times(1);
+  EXPECT_CALL(*mock_bridge_ptr, Show()).Times(1);
+
+  bubble.Update(update_params, base::DoNothing());
+  testing::Mock::VerifyAndClearExpectations(mock_bridge_ptr);
+
+  // In the buggy baseline where ExclusiveAccessBubbleAndroid shadows params_,
+  // ExclusiveAccessBubble::params() returns the frozen base class member,
+  // which still has EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_EXIT_INSTRUCTION
+  // and an empty origin.
+  EXPECT_EQ(bubble.params().type,
+            EXCLUSIVE_ACCESS_BUBBLE_TYPE_KEYBOARD_LOCK_EXIT_INSTRUCTION);
+  EXPECT_EQ(bubble.params().origin, update_params.origin);
+}
+
+TEST_F(ExclusiveAccessBubbleAndroidTest,
+       SnoozeResetPreservesDynamicallyUpdatedParams) {
+  ExclusiveAccessBubbleParams initial_params;
+  initial_params.type =
+      EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_EXIT_INSTRUCTION;
+
+  auto mock_bridge = std::make_unique<MockBridge>();
+  auto* mock_bridge_ptr = mock_bridge.get();
+
+  EXPECT_CALL(*mock_bridge_ptr, IsVisible()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_bridge_ptr, IsKeyboardConnected())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*mock_bridge_ptr, Update(_)).Times(1);
+  EXPECT_CALL(*mock_bridge_ptr, Show()).Times(1);
+
+  auto bubble = std::make_unique<ExclusiveAccessBubbleAndroid>(
+      initial_params, base::DoNothing(), std::move(mock_bridge));
+  auto* bubble_ptr = bubble.get();
+  testing::Mock::VerifyAndClearExpectations(mock_bridge_ptr);
+
+  // Transition to keyboard lock.
+  ExclusiveAccessBubbleParams lock_params;
+  lock_params.type =
+      EXCLUSIVE_ACCESS_BUBBLE_TYPE_KEYBOARD_LOCK_EXIT_INSTRUCTION;
+
+  EXPECT_CALL(*mock_bridge_ptr, IsVisible()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_bridge_ptr, IsKeyboardConnected())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*mock_bridge_ptr, Update(_)).Times(1);
+  EXPECT_CALL(*mock_bridge_ptr, Show()).Times(1);
+
+  bubble_ptr->Update(lock_params, base::DoNothing());
+  testing::Mock::VerifyAndClearExpectations(mock_bridge_ptr);
+
+  ExclusiveAccessContextAndroid context;
+  context.SetBubbleForTesting(std::move(bubble));
+
+  for (int i = 1; i <= 9; ++i) {
+    context.OnExclusiveAccessUserInput();
+  }
+
+  // The 10th input forces a re-show by reading bubble->params().
+  // If params_ is shadowed, bubble->params() returns the initial FULLSCREEN
+  // type, overwriting the live KEYBOARD_LOCK type and sending the wrong exit
+  // text to the bridge.
+  EXPECT_CALL(*mock_bridge_ptr, IsVisible()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_bridge_ptr, IsKeyboardConnected())
+      .WillRepeatedly(Return(true));
+  std::u16string expected_text =
+      exclusive_access_bubble::GetInstructionTextForType(
+          EXCLUSIVE_ACCESS_BUBBLE_TYPE_KEYBOARD_LOCK_EXIT_INSTRUCTION,
+          l10n_util::GetStringUTF16(IDS_APP_ESC_KEY), std::nullopt,
+          /*has_download=*/false, /*notify_overridden=*/false);
+  EXPECT_CALL(*mock_bridge_ptr, Update(expected_text)).Times(1);
+  EXPECT_CALL(*mock_bridge_ptr, Show()).Times(1);
+
+  context.OnExclusiveAccessUserInput();
+  testing::Mock::VerifyAndClearExpectations(mock_bridge_ptr);
+}
+
 }  // namespace

diff --git a/chrome/browser/ui/android/exclusive_access/exclusive_access_bubble_android_unittest.cc b/chrome/browser/ui/android/exclusive_access/exclusive_access_bubble_android_unittest.cc
index 976f848..2be564f 100644
--- a/chrome/browser/ui/android/exclusive_access/exclusive_access_bubble_android_unittest.cc
+++ b/chrome/browser/ui/android/exclusive_access/exclusive_access_bubble_android_unittest.cc
@@ -14,6 +14,10 @@
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/strings/grit/ui_strings.h"
+#include "url/gurl.h"
+#include "url/origin.h"
 
 using testing::_;
 using testing::Return;
@@ -152,4 +156,108 @@
   testing::Mock::VerifyAndClearExpectations(mock_bridge_ptr);
 }
 
+TEST_F(ExclusiveAccessBubbleAndroidTest,
+       ParamsAccessorReturnsLiveUpdatedParams) {
+  ExclusiveAccessBubbleParams initial_params;
+  initial_params.type =
+      EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_EXIT_INSTRUCTION;
+
+  auto mock_bridge = std::make_unique<MockBridge>();
+  auto* mock_bridge_ptr = mock_bridge.get();
+
+  EXPECT_CALL(*mock_bridge_ptr, IsVisible()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_bridge_ptr, IsKeyboardConnected()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_bridge_ptr, Update(_)).Times(1);
+  EXPECT_CALL(*mock_bridge_ptr, Show()).Times(1);
+
+  ExclusiveAccessBubbleAndroid bubble(initial_params, base::DoNothing(),
+                                      std::move(mock_bridge));
+  testing::Mock::VerifyAndClearExpectations(mock_bridge_ptr);
+
+  EXPECT_EQ(bubble.params().type,
+            EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_EXIT_INSTRUCTION);
+
+  // Dynamically update the bubble params (e.g. keyboard lock acquired).
+  ExclusiveAccessBubbleParams update_params;
+  update_params.type =
+      EXCLUSIVE_ACCESS_BUBBLE_TYPE_KEYBOARD_LOCK_EXIT_INSTRUCTION;
+  update_params.origin = url::Origin::Create(GURL("https://example.com"));
+
+  EXPECT_CALL(*mock_bridge_ptr, IsVisible()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_bridge_ptr, IsKeyboardConnected()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_bridge_ptr, Update(_)).Times(1);
+  EXPECT_CALL(*mock_bridge_ptr, Show()).Times(1);
+
+  bubble.Update(update_params, base::DoNothing());
+  testing::Mock::VerifyAndClearExpectations(mock_bridge_ptr);
+
+  // In the buggy baseline where ExclusiveAccessBubbleAndroid shadows params_,
+  // ExclusiveAccessBubble::params() returns the frozen base class member,
+  // which still has EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_EXIT_INSTRUCTION
+  // and an empty origin.
+  EXPECT_EQ(bubble.params().type,
+            EXCLUSIVE_ACCESS_BUBBLE_TYPE_KEYBOARD_LOCK_EXIT_INSTRUCTION);
+  EXPECT_EQ(bubble.params().origin, update_params.origin);
+}
+
+TEST_F(ExclusiveAccessBubbleAndroidTest,
+       SnoozeResetPreservesDynamicallyUpdatedParams) {
+  ExclusiveAccessBubbleParams initial_params;
+  initial_params.type =
+      EXCLUSIVE_ACCESS_BUBBLE_TYPE_FULLSCREEN_EXIT_INSTRUCTION;
+
+  auto mock_bridge = std::make_unique<MockBridge>();
+  auto* mock_bridge_ptr = mock_bridge.get();
+
+  EXPECT_CALL(*mock_bridge_ptr, IsVisible()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_bridge_ptr, IsKeyboardConnected())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*mock_bridge_ptr, Update(_)).Times(1);
+  EXPECT_CALL(*mock_bridge_ptr, Show()).Times(1);
+
+  auto bubble = std::make_unique<ExclusiveAccessBubbleAndroid>(
+      initial_params, base::DoNothing(), std::move(mock_bridge));
+  auto* bubble_ptr = bubble.get();
+  testing::Mock::VerifyAndClearExpectations(mock_bridge_ptr);
+
+  // Transition to keyboard lock.
+  ExclusiveAccessBubbleParams lock_params;
+  lock_params.type =
+      EXCLUSIVE_ACCESS_BUBBLE_TYPE_KEYBOARD_LOCK_EXIT_INSTRUCTION;
+
+  EXPECT_CALL(*mock_bridge_ptr, IsVisible()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_bridge_ptr, IsKeyboardConnected())
+      .WillRepeatedly(Return(true));
+  EXPECT_CALL(*mock_bridge_ptr, Update(_)).Times(1);
+  EXPECT_CALL(*mock_bridge_ptr, Show()).Times(1);
+
+  bubble_ptr->Update(lock_params, base::DoNothing());
+  testing::Mock::VerifyAndClearExpectations(mock_bridge_ptr);
+
+  ExclusiveAccessContextAndroid context;
+  context.SetBubbleForTesting(std::move(bubble));
+
+  for (int i = 1; i <= 9; ++i) {
+    context.OnExclusiveAccessUserInput();
+  }
+
+  // The 10th input forces a re-show by reading bubble->params().
+  // If params_ is shadowed, bubble->params() returns the initial FULLSCREEN
+  // type, overwriting the live KEYBOARD_LOCK type and sending the wrong exit
+  // text to the bridge.
+  EXPECT_CALL(*mock_bridge_ptr, IsVisible()).WillOnce(Return(false));
+  EXPECT_CALL(*mock_bridge_ptr, IsKeyboardConnected())
+      .WillRepeatedly(Return(true));
+  std::u16string expected_text =
+      exclusive_access_bubble::GetInstructionTextForType(
+          EXCLUSIVE_ACCESS_BUBBLE_TYPE_KEYBOARD_LOCK_EXIT_INSTRUCTION,
+          l10n_util::GetStringUTF16(IDS_APP_ESC_KEY), std::nullopt,
+          /*has_download=*/false, /*notify_overridden=*/false);
+  EXPECT_CALL(*mock_bridge_ptr, Update(expected_text)).Times(1);
+  EXPECT_CALL(*mock_bridge_ptr, Show()).Times(1);
+
+  context.OnExclusiveAccessUserInput();
+  testing::Mock::VerifyAndClearExpectations(mock_bridge_ptr);
+}
+
 }  // namespace