Overview

Low
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInappropriate Implementation in Selection
DescriptionInappropriate Implementation in Selection
ComponentSelection
Bug ClassLogic Error
Tracker384033062
Fix commit9cf22ff04513 (chromium/src) +23/-0
CISA KEVNot listed
CreditedAlesandro Ortiz
Disclosed2025-03-04

Files Changed

  • content/browser/android/selection/selection_popup_controller.cc
  • content/public/android/java/src/org/chromium/content/browser/selection/SelectionPopupControllerImpl.java
From 9cf22ff04513c044411467ef06340e3354ccb468 Mon Sep 17 00:00:00 2001
From: Alesandro Ortiz <alesandro@alesandroortiz.com>
Date: Fri, 10 Jan 2025 15:46:09 -0800
Subject: [PATCH] Android: Dismiss text magnifier on RWHVA change

Due to stale states and early returns, `handleDragStopped()` was not
called after certain cross-process navigations. This meant text
magnifier did not always dismiss when renderer process changed.
See bug for more details.

To fix this, we call `handleDragStopped()` when
`SPC::UpdateRenderProcessConnection()` is called to dismiss the text
magnifier. This ensures dismissal after renderer process changes.

Fixed: 384033062
Change-Id: I158529d40dba86f21b275c7031531797498635dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6093444
Reviewed-by: Jinsuk Kim <jinsukkim@chromium.org>
Commit-Queue: Alesandro Ortiz <alesandro@alesandroortiz.com>
Reviewed-by: Bo Liu <boliu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1405064}
---

diff --git a/content/browser/android/selection/selection_popup_controller.cc b/content/browser/android/selection/selection_popup_controller.cc
index 91e6f41..f511d0e 100644
--- a/content/browser/android/selection/selection_popup_controller.cc
+++ b/content/browser/android/selection/selection_popup_controller.cc
@@ -9,6 +9,7 @@
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
+#include "base/feature_list.h"
 #include "content/browser/android/selection/composited_touch_handle_drawable.h"
 #include "content/browser/gpu/gpu_data_manager_impl.h"
 #include "content/browser/renderer_host/render_widget_host_view_android.h"
@@ -63,6 +64,10 @@
   return enabled;
 }
 
+BASE_FEATURE(kDismissMagnifierOnViewSwap,
+             "DismissMagnifierOnViewSwap",
+             base::FEATURE_ENABLED_BY_DEFAULT);
+
 }  // namespace
 
 static jboolean
@@ -200,6 +205,17 @@
   if (new_rwhva)
     new_rwhva->set_selection_popup_controller(this);
   rwhva_ = new_rwhva;
+
+  if (!base::FeatureList::IsEnabled(kDismissMagnifierOnViewSwap)) {
+    return;
+  }
+  JNIEnv* env = AttachCurrentThread();
+  ScopedJavaLocalRef<jobject> obj = java_obj_.get(env);
+  if (obj.is_null()) {
+    return;
+  }
+
+  Java_SelectionPopupControllerImpl_renderWidgetHostViewChanged(env, obj);
 }
 
 void SelectionPopupController::OnSelectionEvent(
diff --git a/content/public/android/java/src/org/chromium/content/browser/selection/SelectionPopupControllerImpl.java b/content/public/android/java/src/org/chromium/content/browser/selection/SelectionPopupControllerImpl.java
index 1f75933..52bd611 100644
--- a/content/public/android/java/src/org/chromium/content/browser/selection/SelectionPopupControllerImpl.java
+++ b/content/public/android/java/src/org/chromium/content/browser/selection/SelectionPopupControllerImpl.java
@@ -1503,6 +1503,13 @@
         }
     }
 
+    @CalledByNative
+    private void renderWidgetHostViewChanged() {
+        if (getMagnifierAnimator() != null) {
+            getMagnifierAnimator().handleDragStopped();
+        }
+    }
+
     // All coordinates are in DIP.
     @VisibleForTesting
     @CalledByNative
Loading diff…

Original Bug Report

reported by al...@alesandroortiz.com

Security: Android selection magnifier persists after navigation, can obscure/spoof browser UI or page content

SUMMARY

The Android text selection magnifier remains on screen with attacker-controlled content after navigating away from attacker page. This can result in browser UI or another site’s content being obscured or spoofed.

It’s a detailed report with proposed patch, but likely low/medium severity at best, so please enjoy your holiday breaks.

VULNERABILITY DETAILS

The Android text selection magnifier is a rectangular box that shows the page content around a selection or insertion cursor/caret. The magnifier is shown when a user taps and holds a text handle. Text handles are shown when a user selects text in page or input field, or when there is an insertion caret in input fields or other content-editable elements.

When navigating to another page, Chromium will close an open magnifier in these cases:

  • When a page load starts (but only after commit 3c58f9355 in October 2024, see Additional Context section)
  • When a page load ends (if TouchSelectionController (TSC) hasn’t changed, i.e. in most same-site navigations)
  • When user lifts finger from text handle before page load ends

However, the magnifier will remain open with content from the previous page if:

  • Page load starts, then user opens magnifier, then page load ends while magnifier is open. However, this only occurs if a new TouchSelectionController instance is created on page load start, which reliably occurs in cross-site navigations (see Root Cause section).

When the page load ends, the magnifier content stops being updated. The content will be the last content rendered on the previous page, which is attacker controlled. The magnifier will render over any page content or most browser UI, including the address bar and permission prompts.

Therefore, an attacker can either obscure or spoof browser UI or another page’s content. For example, an attacker can show text over the address bar, show text/images or obscure buttons in permission prompts, or show text/images over another site’s page.

The magnifier will remain shown until the user switches away from the Chromium browser to another app or turns off their screen. Opening a new magnifier does NOT dismiss the existing magnifier, so this can be repeated multiple times to obscure/spoof a larger area.

VERSION

Chrome Version: 131.0.6778.105 Stable, 133.0.6889.0 Canary

Repros down to 69.0.3474.0 with #enable-site-per-process flag, 78.0.3877.0 without flags.

Operating System: Android 14, Android 12

BISECT

There’s a couple of bisects. These bisects were made with the PoC that opens magnifier before page load starts (android-magnifier-minimal-no-delay.html).

  1. Without any overridden flags: https://crrev.com/ab7700c387f9167d763484cfa659ef7931103890

    Enable ProactivelySwapBrowsingInstance in fieldtrial_testing_config (August 2019)

    If there are other attacker-controllable conditions that force a RWHVA/TSC swap, then it may repro earlier without flags.

  2. With #enable-site-per-process enabled: https://crrev.com/2d7e42f08f486ca3d5e28176f5ea0e3fb5bde0e8 (June 2018)

    This commit added the Magnifier logic for Android.

ADDITIONAL CONTEXT

Prior to commit 3c58f9355 [1] (October 2024), the magnifier will also remain open if:

  • User opens magnifier, then page load starts, then page load ends while magnifier is open. Only if a new TouchSelectionController instance is created on page load start.

PoC and video are also provided for before commit 3c58f9355, see android-magnifier-minimal-no-delay.html. I verified the older behavior via bisect. Also verified separately by commenting out ClearSelection() call from onPageLoadStarted() in ToT local build.

After the commit, the onPageLoadStarted() [2] observer closes the magnifier by calling SelectionPopupController::ClearSelection() [3] when a navigation is started. This is why after the commit a user must open the magnifier after the navigation starts. Before the commit, there is no ClearSelection() call on page load start.

[1] https://crrev.com/3c58f935519ca251c7ab32a423b66bd055c957ad [dnt] Back gesture clears text selection Landed in 131.0.6766.0 on October 8th, and merged into 131.0.6778.39 Stable.

[2] onPageLoadStarted() https://source.chromium.org/chromium/chromium/src/+/main:chrome/android/java/src/org/chromium/chrome/browser/ChromeActionModeHandler.java;l=102;drc=a0108592277483168bcedfd90b182a8b8ca5c5de

[3] ClearSelection() https://source.chromium.org/chromium/chromium/src/+/main:content/public/android/java/src/org/chromium/content/browser/selection/SelectionPopupControllerImpl.java;l=1701;drc=616d60fca655937c2b730db94fd32d37ddff3bb5

REPRODUCTION CASE

Minimal PoCs don’t require any setup. Realistic PoCs use a signal server which requires some setup.

Minimal PoC

This PoC is for current versions, after commit 3c58f9355. The magnifier must be opened after the navigation starts but before the nav finishes.

  1. Navigate to https://alesandroortiz.com/security/chromium/android-magnifier-minimal.html
  2. Tap an input field once, then touch and hold the text handle, then wait a few seconds for slow navigation to finish.

Observed:

  • For top input field: Magnifier remains open over address bar. Address bar is partially obscured by magnifier.
  • For center input field: Magnifier remains open over next page. The next page’s content is partially obscured by magnifier.

Expected: Magnifier is dismissed when user stops holding text handle or when next page finishes loading.

Minimal PoC, before commit 3c58f9355

This PoC works only before commit 3c58f9355 (r1365650). The magnifier can be opened before the navigation starts, so attack is easier to setup and perform.

  1. Navigate to https://alesandroortiz.com/security/chromium/android-magnifier-minimal-no-delay.html
  2. Tap an input field once, then touch and move the text handle, then wait a few moments for navigation to occur.

Observed/Expected: Same as previous Minimal PoC.

Setup for self-hosting realistic PoCs:

The realistic PoCs use a signal server to avoid longer-than-needed navigation delay after magnifier is opened. For best results, the signal server should use HTTPS.

Signal server requires attached signal-server.js + permission.html + spoof.html

(If initial page is HTTPS, then signal server also requires HTTPS to avoid fetch being blocked due to mixed content. For permission prompt PoC, signal server must be on HTTPS due to permissions API requiring secure context.)

Important: The initial page and signal server MUST be on different sites for reliable repro, due to TSC swap requirement. Different ports on same hostname work fine for this purpose.

Setup HTTPS for signal server:

  1. Generate cert (self-signed: openssl req -x509 -nodes -newkey rsa:4096 -keyout key.pem -out cert.pem -sha256 -days 365), then update signal-server.js to use the key pair in https.createServer() and disable the http.createServer() call.

Before running PoCs:

  1. Run signal server. If using self-signed/untrusted cert, visit https://signal-server-host:port/ once to accept unsafe connection warning.
  2. Update SIGNAL_SERVER variable in initiator pages to reference your signal server (instead of the https://aogarantiza.com:1337 signal server, which won’t work for you)
  3. Host initiator page on different site than signal server (e.g. using npx http-server locally; can be on HTTP)

Address bar spoof PoC

Note: Ensure signal server is running and is on a different site than initiator page.

  1. Navigate to initiator page: android-magnifier-omnibox.html
  2. Tap or slide finger anywhere.
  3. Slide finger as instructed by page.

Observed: Magnifier remains open over address bar. Address bar is partially obscured by magnifier.

Expected: Magnifier is dismissed when user stops holding text handle or when next page finishes loading.

Permission prompt obscured button PoC

Note: Signal server must be on HTTPS. Ensure signal server is running and is on a different site than initiator page.

  1. Navigate to intiator page: android-magnifier-permission.html
  2. Tap or slide finger anywhere.
  3. Slide finger as instructed by page.
  4. Tap anywhere when instructed by page to open permission prompt.

Observed: Magnifier remains open over permission prompt. Permission prompt is partially obscured by magnifier.

Expected: Same as previous PoC.

ROOT CAUSE

Observations below based on logs.

In cases where the RenderWidgetHostViewAndroid (RWHVA) [1] and TouchSelectionController (TSC) [2] instances stay alive across navigations, the magnifier is closed as expected when navigation finishes. When new instances of RWHVA and TSC are created, the magnifier remains open after navigation finishes.

I’ve observed that RWHVA and TSC are swapped for all cross-site navigations, and for some same-site navigations. Therefore, a cross-site navigation is an easy way to reliably reach the trigger conditions.

When a navigation commits, the renderer calls LayerTreeHostImpl::GenerateCompositorFrame() [5] which sends RenderFrameMetadata (RFM) to the browser. The RFM contains selection data.

The RFM selection data is calculated by LayerTreeImpl::GetViewportSelection() [3] (called by LayerTreeHostImpl::MakeRenderFrameMetadata() [4]). When a page loads, the RFM will indicate an empty selection (i.e. no selection) even when there was a selection in the previous page. This is expected behavior since the new page does not have a selection.

The RFM with selection data goes through this code path:

  • RFM created: LayerTreeHostImpl::GenerateCompositorFrame() [5]
  • RenderFrameMetadataObserverImpl::OnRenderFrameSubmission() [6] (renderer)
  • RenderFrameMetadataProviderImpl::OnRenderFrameMetadataChanged() [7] (browser)
  • RenderWidgetHostViewAndroid::OnRenderFrameMetadataChangedBeforeActivation() [8]
  • RenderWidgetHostViewAndroid::UpdateTouchSelectionController() [9]
  • TouchSelectionControllerClientManagerAndroid::UpdateClientSelectionBounds() [10]
  • TouchSelectionController::OnSelectionBoundsChanged() [11]

There is a key difference in behavior in TSC::OnSelectionBoundsChanged() between the repro vs. non-repro scenarios.

  • Repro (unexpected): For a newly-created TSC, the TSC’s internal state will be initialized to empty/no selection. If the RFM also has an empty selection, TSC::OnSelectionBoundsChanged() will return early due to start == start_ && end_ == end being true.
  • Non-repro (expected): The existing TSC will have selection data from the previous page. Since the selection has changed compared to the selection previously known to the TSC, the rest of the OnSelectionBoundsChanged() logic will run.

In non-repro (expected) cases, the code path continues from OnSelectionBoundsChanged()

  • TouchSelectionController::OnSelectionBoundsChanged() [11] updates the internal selection state
  • TouchSelectionController::HideHandles() (start/end.HasHandle() are false)
  • TouchSelectionController::DeactivateInsertion()
  • TouchHandle::SetEnabled(false)
  • TouchHandle::EndDrag()
  • TouchSelectionController::OnDragEnd()
  • RenderWidgetHostViewAndroid::OnSelectionEvent(INSERTION_HANDLE_DRAG_STOPPED)
  • SelectionPopupController::OnSelectionEvent() (C++ -> Java) with SelectionEventType.INSERTION_HANDLE_DRAG_STOPPED
  • MagnifierAnimator::handleDragStopped()
  • MagnifierWrapper::dismiss() / MagnifierSurfaceControl::dismiss() which finally dismisses the Android magnifier.

The calls after TSC::OnSelectionBoundsChanged() are easier to follow, so please use the Code Search link [11] to find the functions mentioned above.

Given the above, the magnifier isn’t cleared in the repro cases because:

  • RWHVA and TSC change on navigation start, and are initialized as having no selection. TSC only dismisses the Magnifier if it knew of a previous selection and the new state is no selection.
  • There are no other calls to MagnifierWrapper::dismiss()

[1] RenderWidgetHostViewAndroid https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/render_widget_host_view_android.cc;drc=0f0c2e5c34d04fb5875b26061fc349152787dd2e

[2] TouchSelectionController https://source.chromium.org/chromium/chromium/src/+/main:ui/touch_selection/touch_selection_controller.cc;drc=5f8dea3b512f8d1769e0d5c83a1721beedd0b458

[3] LayerTreeImpl::GetViewportSelection() https://source.chromium.org/chromium/chromium/src/+/main:cc/trees/layer_tree_impl.cc;l=2878;drc=616d60fca655937c2b730db94fd32d37ddff3bb5

[4] LayerTreeHostImpl::MakeRenderFrameMetadata() https://source.chromium.org/chromium/chromium/src/+/main:cc/trees/layer_tree_host_impl.cc;l=2591;drc=616d60fca655937c2b730db94fd32d37ddff3bb5

[5] LayerTreeHotImpl::GenerateCompositorFrame() calls OnRenderFrameSubmission() https://source.chromium.org/chromium/chromium/src/+/main:cc/trees/layer_tree_host_impl.cc;l=3059;drc=616d60fca655937c2b730db94fd32d37ddff3bb5

[6] RenderFrameMetadataObserverImpl::OnRenderFrameSubmission() https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/widget/compositing/render_frame_metadata_observer_impl.cc;l=100;drc=616d60fca655937c2b730db94fd32d37ddff3bb5

[7] RenderFrameMetadataProviderImpl::OnRenderFrameMetadataChanged() https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/render_frame_metadata_provider_impl.cc;l=118;drc=54fec6df88aed90af1239ebbf49f5deced265e8d

[8] RenderWidgetHostViewAndroid::OnRenderFrameMetadataChangedBeforeActivation() https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/render_widget_host_view_android.cc;l=879;drc=616d60fca655937c2b730db94fd32d37ddff3bb5

[9] RenderWidgetHostViewAndroid::UpdateTouchSelectionController() https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/render_widget_host_view_android.cc;l=1936;drc=616d60fca655937c2b730db94fd32d37ddff3bb5

[10] TouchSelectionControllerClientManagerAndroid::UpdateClientSelectionBounds() https://source.chromium.org/chromium/chromium/src/+/main:content/browser/renderer_host/input/touch_selection_controller_client_manager_android.cc;l=64;drc=616d60fca655937c2b730db94fd32d37ddff3bb5

[11] TouchSelectionController::OnSelectionBoundsChanged() https://source.chromium.org/chromium/chromium/src/+/main:ui/touch_selection/touch_selection_controller.cc;l=77;drc=616d60fca655937c2b730db94fd32d37ddff3bb5

PATCH

The proposed patch to fix the issue adds a onDidFinishNavigationInPrimaryMainFrame() observer in ChromeActionModeHandler that makes a call to SelectionPopupController::ClearSelection(). This follows the same pattern used to clear the selection on page load start via onPageLoadStarted() observer.

The patch also adds a MagnifierAnimator::handleDragStopped() call within SelectionPopupController::ClearSelection().

I scoped the fix to only Java code because existing code paths that lead to MagnifierAnimator::handleDragStopped() are short-circuited in multiple places in the C++ code, as shown in Root Cause section. Updating the C++ code path to make the Java call may cause unexpected behavior changes elsewhere, since the code path involves many layers and is shared with other scenarios, not just the page load finished scenario.

As far as I can tell, there are no unexpected behavior changes with this patch.

Behavior after applying patch is shown in videos named fixed-*.

Credit Information

Reporter credit: Alesandro Ortiz https://AlesandroOrtiz.com

View on issue tracker
Links in the report