From 5d2f26ffac8bde6283e02e28ea61ae11c5b0affe Mon Sep 17 00:00:00 2001
From: Yi Gu <yigu@chromium.org>
Date: Thu, 10 Sep 2026 08:00:36 -0700
Subject: [PATCH] [WebOTP] Sanitize OTP & isolate BiDi strings in SMSFetcher notification

When rendering the cross-device WebOTP notification on Android, an
attacker-crafted OTP containing bidirectional formatting controls (e.g.
U+202E RLO) or excessive whitespace (such as NBSP runs) could visually
reorder, rewrite, or push off-screen adjacent security-relevant text,
such as the client device name or origin.

This CL addresses the issue at the presentation layer:
1. Strips Unicode control (Cc) and format (Cf) characters from the OTP
   for display, collapses whitespace/separators, and caps display length.
2. Wraps all interpolated strings (OTP, client name, origins) with
   BidiFormatter to prevent directional bleed into the surrounding text.
3. Preserves the authentic OTP and origins for backend processing and
   JNI callbacks without modifying SmsParser::Parse().

Fixed: 514204338
Change-Id: I21e29c2d54155f611a53e7b0da4c9289fbd39709
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8377828
Reviewed-by: Hira Mahmood <hiramahmood@google.com>
Commit-Queue: Yi Gu <yigu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1695447}
---
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/sharing/sms_fetcher/SmsFetcherMessageHandler.java b/chrome/android/java/src/org/chromium/chrome/browser/sharing/sms_fetcher/SmsFetcherMessageHandler.java
index 577afa4e..0873e41 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/sharing/sms_fetcher/SmsFetcherMessageHandler.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/sharing/sms_fetcher/SmsFetcherMessageHandler.java
@@ -10,6 +10,9 @@
 import android.content.Intent;
 import android.content.res.Resources;
 
+import androidx.annotation.VisibleForTesting;
+import androidx.core.text.BidiFormatter;
+
 import org.jni_zero.CalledByNative;
 import org.jni_zero.JniType;
 import org.jni_zero.NativeMethods;
@@ -38,6 +41,30 @@
     private static @Nullable String sTopOrigin;
     private static @Nullable String sEmbeddedOrigin;
 
+    @VisibleForTesting static final int MAX_OTP_DISPLAY_LENGTH = 32;
+
+    /**
+     * Sanitizes the one-time code for safe display in the notification title. Strips Unicode
+     * control characters (Cc) and formatting characters (Cf, which includes BiDi overrides like
+     * RLO, LRO, FSI, PDI), collapses whitespace runs and Unicode separators (including NBSP
+     * U+00A0), and caps the maximum length.
+     *
+     * @param oneTimeCode The raw one time code from SMS.
+     * @return The sanitized code safe for UI display.
+     */
+    @VisibleForTesting
+    static String sanitizeOneTimeCodeForDisplay(String oneTimeCode) {
+        if (oneTimeCode == null) return "";
+        // Strip control characters (Cc) and formatting characters (Cf, including BiDi overrides).
+        String sanitized = oneTimeCode.replaceAll("[\\p{Cc}\\p{Cf}]", "");
+        // Collapse whitespace runs and Unicode separators (e.g. NBSP U+00A0, thin space, etc.).
+        sanitized = sanitized.replaceAll("[\\p{Z}\\s]+", " ").trim();
+        if (sanitized.length() > MAX_OTP_DISPLAY_LENGTH) {
+            sanitized = sanitized.substring(0, MAX_OTP_DISPLAY_LENGTH) + "…";
+        }
+        return sanitized;
+    }
+
     /** Handles the interaction of an incoming notification when an expected SMS arrives. */
     public static final class NotificationReceiver extends BroadcastReceiver {
         @Override
@@ -75,29 +102,37 @@
      * @param oneTimeCode The one time code from SMS
      * @param topOrigin The top frame origin from the SMS
      * @param embeddedOrigin The embedded frame origin from the SMS. Null if the SMS does not
-     *         contain an iframe origin.
+     *     contain an iframe origin.
      * @param clientName The client name where the remote request comes from
      */
-    private static String getNotificationTitle(
+    @VisibleForTesting
+    static String getNotificationTitle(
             String oneTimeCode,
             String topOrigin,
             @Nullable String embeddedOrigin,
             String clientName) {
         Resources resources = ContextUtils.getApplicationContext().getResources();
+        BidiFormatter bidi = BidiFormatter.getInstance();
+        String safeOtp = bidi.unicodeWrap(sanitizeOneTimeCodeForDisplay(oneTimeCode));
+        String safeClientName = bidi.unicodeWrap(clientName);
+        String safeTopOrigin = bidi.unicodeWrap(topOrigin);
+        String safeEmbeddedOrigin =
+                embeddedOrigin == null ? null : bidi.unicodeWrap(embeddedOrigin);
+
         if (ChromeFeatureList.isEnabled(ChromeFeatureList.WEB_OTP_CROSS_DEVICE_SIMPLE_STRING)) {
-            if (embeddedOrigin == null) {
+            if (safeEmbeddedOrigin == null) {
                 return resources.getString(
                         R.string.sms_fetcher_notification_title_simple_string,
-                        oneTimeCode,
-                        topOrigin);
+                        safeOtp,
+                        safeTopOrigin);
             }
             return resources.getString(
                     R.string.sms_fetcher_notification_title_simple_string,
-                    oneTimeCode,
-                    embeddedOrigin);
+                    safeOtp,
+                    safeEmbeddedOrigin);
         }
         return resources.getString(
-                R.string.sms_fetcher_notification_title, oneTimeCode, clientName);
+                R.string.sms_fetcher_notification_title, safeOtp, safeClientName);
     }
 
     /**
@@ -108,19 +143,26 @@
      *     contain an iframe origin.
      * @param clientName The client name where the remote request comes from
      */
-    private static String getNotificationText(
+    @VisibleForTesting
+    static String getNotificationText(
             String topOrigin, @Nullable String embeddedOrigin, String clientName) {
         Resources resources = ContextUtils.getApplicationContext().getResources();
+        BidiFormatter bidi = BidiFormatter.getInstance();
+        String safeTopOrigin = bidi.unicodeWrap(topOrigin);
+        String safeClientName = bidi.unicodeWrap(clientName);
+        String safeEmbeddedOrigin =
+                embeddedOrigin == null ? null : bidi.unicodeWrap(embeddedOrigin);
+
         if (ChromeFeatureList.isEnabled(ChromeFeatureList.WEB_OTP_CROSS_DEVICE_SIMPLE_STRING)) {
-            if (embeddedOrigin == null) return clientName;
-            return topOrigin + " · " + clientName;
+            if (safeEmbeddedOrigin == null) return safeClientName;
+            return safeTopOrigin + " · " + safeClientName;
         }
-        return embeddedOrigin == null
-                ? resources.getString(R.string.sms_fetcher_notification_text, topOrigin)
+        return safeEmbeddedOrigin == null
+                ? resources.getString(R.string.sms_fetcher_notification_text, safeTopOrigin)
                 : resources.getString(
                         R.string.sms_fetcher_notification_text_for_embedded_frame,
-                        topOrigin,
-                        embeddedOrigin);
+                        safeTopOrigin,
+                        safeEmbeddedOrigin);
     }
 
     /**
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/sharing/sms_fetcher/SmsFetcherMessageHandlerTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/sharing/sms_fetcher/SmsFetcherMessageHandlerTest.java
new file mode 100644
index 0000000..66fcee8
--- /dev/null
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/sharing/sms_fetcher/SmsFetcherMessageHandlerTest.java
@@ -0,0 +1,158 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.sharing.sms_fetcher;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import android.content.Context;
+
+import androidx.test.core.app.ApplicationProvider;
+import androidx.test.filters.SmallTest;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.chromium.base.ContextUtils;
+import org.chromium.base.test.BaseRobolectricTestRunner;
+import org.chromium.base.test.util.Features.DisableFeatures;
+import org.chromium.base.test.util.Features.EnableFeatures;
+import org.chromium.chrome.browser.flags.ChromeFeatureList;
+
+/** Unit tests for {@link SmsFetcherMessageHandler}. */
+@RunWith(BaseRobolectricTestRunner.class)
+public class SmsFetcherMessageHandlerTest {
+    private Context mContext;
+
+    @Before
+    public void setUp() {
+        mContext = ApplicationProvider.getApplicationContext();
+        ContextUtils.initApplicationContextForTests(mContext);
+    }
+
+    @Test
+    @SmallTest
+    public void testSanitizeOneTimeCodeForDisplay_normalCode() {
+        assertEquals("123456", SmsFetcherMessageHandler.sanitizeOneTimeCodeForDisplay("123456"));
+        assertEquals("ABC-123", SmsFetcherMessageHandler.sanitizeOneTimeCodeForDisplay("ABC-123"));
+    }
+
+    @Test
+    @SmallTest
+    public void testSanitizeOneTimeCodeForDisplay_nullOrEmpty() {
+        assertEquals("", SmsFetcherMessageHandler.sanitizeOneTimeCodeForDisplay(null));
+        assertEquals("", SmsFetcherMessageHandler.sanitizeOneTimeCodeForDisplay(""));
+        assertEquals("", SmsFetcherMessageHandler.sanitizeOneTimeCodeForDisplay("   "));
+    }
+
+    @Test
+    @SmallTest
+    public void testSanitizeOneTimeCodeForDisplay_stripsBiDiControls() {
+        // U+202E (RLO - Right-to-Left Override)
+        assertEquals(
+                "951753", SmsFetcherMessageHandler.sanitizeOneTimeCodeForDisplay("951753\u202E"));
+        // Multiple BiDi controls: LRE, RLE, PDF, LRO, RLO, LRI, RLI, FSI, PDI
+        String malicious = "951753\u202A\u202B\u202C\u202D\u202E\u2066\u2067\u2068\u2069";
+        assertEquals("951753", SmsFetcherMessageHandler.sanitizeOneTimeCodeForDisplay(malicious));
+    }
+
+    @Test
+    @SmallTest
+    public void testSanitizeOneTimeCodeForDisplay_stripsControlCharacters() {
+        assertEquals(
+                "123456",
+                SmsFetcherMessageHandler.sanitizeOneTimeCodeForDisplay("12\u000034\u001F56"));
+    }
+
+    @Test
+    @SmallTest
+    public void testSanitizeOneTimeCodeForDisplay_collapsesWhitespaceAndNBSP() {
+        // Non-breaking spaces (U+00A0)
+        assertEquals(
+                "951753",
+                SmsFetcherMessageHandler.sanitizeOneTimeCodeForDisplay("951753\u00A0\u00A0"));
+        assertEquals(
+                "951753 code",
+                SmsFetcherMessageHandler.sanitizeOneTimeCodeForDisplay("951753\u00A0\u00A0code"));
+        // Flooding with 100 NBSP characters
+        StringBuilder sb = new StringBuilder("951753");
+        for (int i = 0; i < 100; i++) {
+            sb.append("\u00A0");
+        }
+        assertEquals(
+                "951753", SmsFetcherMessageHandler.sanitizeOneTimeCodeForDisplay(sb.toString()));
+    }
+
+    @Test
+    @SmallTest
+    public void testSanitizeOneTimeCodeForDisplay_truncatesExcessiveLength() {
+        StringBuilder sb = new StringBuilder();
+        for (int i = 0; i < 40; i++) {
+            sb.append("a");
+        }
+        String result = SmsFetcherMessageHandler.sanitizeOneTimeCodeForDisplay(sb.toString());
+        assertEquals(SmsFetcherMessageHandler.MAX_OTP_DISPLAY_LENGTH + 1, result.length());
+        assertTrue(result.endsWith("…"));
+        StringBuilder expected = new StringBuilder();
+        for (int i = 0; i < SmsFetcherMessageHandler.MAX_OTP_DISPLAY_LENGTH; i++) {
+            expected.append("a");
+        }
+        expected.append("…");
+        assertEquals(expected.toString(), result);
+    }
+
+    @Test
+    @SmallTest
+    @DisableFeatures(ChromeFeatureList.WEB_OTP_CROSS_DEVICE_SIMPLE_STRING)
+    public void testGetNotificationTitle_defaultFormat() {
+        String title =
+                SmsFetcherMessageHandler.getNotificationTitle(
+                        "951753\u202E", "example.com", null, "MacBook Pro");
+        assertNotNull(title);
+        // Ensure RLO is stripped
+        assertFalse(title.contains("\u202E"));
+        // Ensure OTP and client name are present
+        assertTrue(title.contains("951753"));
+        assertTrue(title.contains("MacBook Pro"));
+    }
+
+    @Test
+    @SmallTest
+    @EnableFeatures(ChromeFeatureList.WEB_OTP_CROSS_DEVICE_SIMPLE_STRING)
+    public void testGetNotificationTitle_simpleStringFormat() {
+        String title =
+                SmsFetcherMessageHandler.getNotificationTitle(
+                        "951753\u202E", "example.com", null, "MacBook Pro");
+        assertNotNull(title);
+        assertFalse(title.contains("\u202E"));
+        assertTrue(title.contains("951753"));
+        assertTrue(title.contains("example.com"));
+    }
+
+    @Test
+    @SmallTest
+    @DisableFeatures(ChromeFeatureList.WEB_OTP_CROSS_DEVICE_SIMPLE_STRING)
+    public void testGetNotificationText_defaultFormat() {
+        String text =
+                SmsFetcherMessageHandler.getNotificationText("example.com", null, "MacBook Pro");
+        assertNotNull(text);
+        assertTrue(text.contains("example.com"));
+    }
+
+    @Test
+    @SmallTest
+    @DisableFeatures(ChromeFeatureList.WEB_OTP_CROSS_DEVICE_SIMPLE_STRING)
+    public void testGetNotificationText_embeddedFrame() {
+        String text =
+                SmsFetcherMessageHandler.getNotificationText(
+                        "top.com", "embedded.com", "MacBook Pro");
+        assertNotNull(text);
+        assertTrue(text.contains("top.com"));
+        assertTrue(text.contains("embedded.com"));
+    }
+}
diff --git a/chrome/android/BUILD.gn b/chrome/android/BUILD.gn
index 70762a2..b7c71c7 100644
--- a/chrome/android/BUILD.gn
+++ b/chrome/android/BUILD.gn
@@ -519,8 +519,8 @@
       "//chrome/browser/single_tab/android:java",
       "//chrome/browser/ssl/android:java",
       "//chrome/browser/subscription_eligibility:java",
-      "//chrome/browser/supervised_user:extension_parent_approval_java",
       "//chrome/browser/supervised_user:android_parental_controls_bridge_java",
+      "//chrome/browser/supervised_user:extension_parent_approval_java",
       "//chrome/browser/supervised_user:supervised_user_service_platform_delegate_java",
       "//chrome/browser/supervised_user:website_parent_approval_java",
       "//chrome/browser/sync/android:java",
@@ -1152,6 +1152,7 @@
       "junit:chrome_junit_tests_org.chromium.chrome.browser.segmentation_platform",
       "junit:chrome_junit_tests_org.chromium.chrome.browser.settings",
       "junit:chrome_junit_tests_org.chromium.chrome.browser.share",
+      "junit:chrome_junit_tests_org.chromium.chrome.browser.sharing",
       "junit:chrome_junit_tests_org.chromium.chrome.browser.signin",
       "junit:chrome_junit_tests_org.chromium.chrome.browser.suggestions",
       "junit:chrome_junit_tests_org.chromium.chrome.browser.sync",
diff --git a/chrome/android/junit/BUILD.gn b/chrome/android/junit/BUILD.gn
index 383de08..2efee02 100644
--- a/chrome/android/junit/BUILD.gn
+++ b/chrome/android/junit/BUILD.gn
@@ -1206,6 +1206,13 @@
     ]
   }
 
+  robolectric_library(
+      "chrome_junit_tests_org.chromium.chrome.browser.sharing") {
+    sources = [ "src/org/chromium/chrome/browser/sharing/sms_fetcher/SmsFetcherMessageHandlerTest.java" ]
+
+    deps = [ ":chrome_junit_tests_helper" ]
+  }
+
   robolectric_library("chrome_junit_tests_org.chromium.chrome.browser.signin") {
     sources = [
       "src/org/chromium/chrome/browser/signin/ChildAccountStatusSupplierTest.java",