Medium chrome Logic Error 🔧 Commit mapped

Overview

Medium
Severity
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactObservable discrepancy in Safebrowsing
DescriptionObservable discrepancy in Safebrowsing
ComponentSafebrowsing
Bug ClassLogic Error
Tracker517917560
Fix commit74f96d1660aa (chromium/src) +212/-3
CISA KEVNot listed
CreditedGoogle
Disclosed2026-09-08

Changed Functions

FunctionChangeNotes
for
ios/chrome/browser/safe_browsing/model/password_protection_egtest.mm
modified
if
ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.mm
modified

Files Changed

  • ios/chrome/browser/safe_browsing/model/password_protection_egtest.mm
  • ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.h
  • ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.mm
From 74f96d1660aa390f1b30a8473f7f793a36966c57 Mon Sep 17 00:00:00 2001
From: Daniel White <danieltwhite@google.com>
Date: Wed, 09 Sep 2026 12:53:01 -0700
Subject: [PATCH] Reland "[iOS] Rate-limiting KeyDown Events in PhishGuard"

This reverts commit b5be033b1bd6d258750a4c12cee93bb52da1ddef.

Reason for revert: The new rate-limiting implementation works across
WebStates.

Bug: 553164077

Original change's description:
> Revert "[iOS] Rate-limiting KeyDown Events in PhishGuard"
>
> This reverts commit fcdba23ade397f9a8a82508e678efad9f9e6f976.
>
> Reason for revert: This implementation doesn't work across WebStates.
>
> Failure Link: https://g-issues.chromium.org/issues/553164077
>
> Bug: 553164077
>
> Original change's description:
> > [iOS] Rate-limiting KeyDown Events in PhishGuard
> >
> > This change implements rate-limiting for KeyDown events in the
> > PasswordProtectionJavaScriptFeature (PhishGuard) on iOS. This mirrors
> > the existing rate-limiting logic for paste events to prevent security
> > issues from high-frequency input while still allowing for realistic
> > typing speeds.
> >
> > Bug: 517917560
> > Change-Id: I296c028112d72fa66607008cc471c5bf023a3458
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8141302
> > Reviewed-by: jdh <jdh@chromium.org>
> > Commit-Queue: Daniel White <danieltwhite@google.com>
> > Cr-Commit-Position: refs/heads/main@{#1674350}
>
> Bug: 517917560
> Change-Id: Ia4b002a71885fd7e5b141b5ae2921862cddb89ea
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8372426
> Commit-Queue: Daniel White <danieltwhite@google.com>
> Reviewed-by: jdh <jdh@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1694051}

Bug: 553164077
Bug: 517917560
Change-Id: Ia5f1276cc6c54be8452723a9523ef2bc843839b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8377162
Commit-Queue: Daniel White <danieltwhite@google.com>
Reviewed-by: jdh <jdh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1694823}
---

diff --git a/ios/chrome/browser/safe_browsing/model/password_protection_egtest.mm b/ios/chrome/browser/safe_browsing/model/password_protection_egtest.mm
index 5f9037f7..f1fc8c4 100644
--- a/ios/chrome/browser/safe_browsing/model/password_protection_egtest.mm
+++ b/ios/chrome/browser/safe_browsing/model/password_protection_egtest.mm
@@ -8,6 +8,7 @@
 
 #import "base/ios/ios_util.h"
 #import "base/test/ios/wait_util.h"
+#import "base/time/time.h"
 #import "ios/chrome/browser/passwords/model/password_manager_app_interface.h"
 #import "ios/chrome/browser/passwords/password_breach/public/password_breach_constants.h"
 #import "ios/chrome/common/ui/elements/form_input_accessory_view.h"
@@ -189,6 +190,9 @@
 
   [ChromeEarlGrey simulatePhysicalKeyboardEvent:@"P" flags:UIKeyModifierShift];
   for (NSString* character in @[ @"a", @"s", @"s", @"w", @"o", @"r", @"d" ]) {
+    // Keydown events are rate-limited. Without a sufficient delay,
+    // Safe Browsing's Password Protection drops the keydown event.
+    base::PlatformThread::Sleep(base::Milliseconds(100));
     [ChromeEarlGrey simulatePhysicalKeyboardEvent:character flags:0];
   }
 }
diff --git a/ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.h b/ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.h
index b8a89b2f..98cd268 100644
--- a/ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.h
+++ b/ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.h
@@ -57,11 +57,23 @@
   absl::flat_hash_map<web::WebState*, std::unique_ptr<base::OneShotTimer>>
       paste_key_timers_;
 
+  // Maps WebStates to the timestamp of the last allowed keydown event.
+  absl::flat_hash_map<web::WebState*, base::TimeTicks> last_keydown_timestamps_;
+
+  // Process-wide (all WebStates combined) rate-limit window state.
+  base::TimeTicks keydown_interval_start_;
+  int keydown_events_in_interval_ = 0;
+
   // Returns true if a paste event (shortcut or actual paste) for `web_state`
   // should be ignored due to rate limiting. Otherwise, updates the last paste
   // timestamp and returns false.
   bool IsPasteRateLimited(web::WebState* web_state);
 
+  // Returns true if a keydown event for `web_state` should be ignored due to
+  // rate limiting. Otherwise, updates the last keydown timestamp and returns
+  // false.
+  bool IsKeyDownRateLimited(web::WebState* web_state);
+
   // Timer helper methods.
   void StartPasteKeyTimer(web::WebState* web_state);
   void OnPasteKeyTimerExpired(web::WebState* web_state);
diff --git a/ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.mm b/ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.mm
index 67c5ede..54ff9f9 100644
--- a/ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.mm
+++ b/ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.mm
@@ -24,9 +24,38 @@
 const char kPasteEventType[] = "TextPasted";
 const char kPasteKeyDetectedEventType[] = "PasteKeyDetected";
 
+constexpr base::TimeDelta kKeyDownRateLimit = base::Milliseconds(25);
 constexpr base::TimeDelta kPasteRateLimit = base::Milliseconds(200);
 inline constexpr base::TimeDelta kPasteKeyTimerDuration =
     base::Milliseconds(100);
+
+// Process-wide aggregate budget, counted across all WebStates. Only one
+// WebState receives keyboard or paste input at a time, so normal input
+// does not exceed a single tab's budget (40 keydown/s, 5 paste/s); the caps
+// below provide additional headroom for focus transitions, iPad multi-window
+// environments, and burst typing while bounding aggregate event frequency.
+constexpr base::TimeDelta kAggregateRateLimitInterval = base::Seconds(1);
+constexpr int kMaxKeyDownEventsPerInterval = 80;
+
+// Returns true if an additional event should be dropped because the
+// process-wide budget (`max_events_per_interval` per aggregate interval) is
+// exhausted; otherwise consumes one slot and returns false.
+bool IsAggregateRateLimited(base::TimeTicks now,
+                            base::TimeTicks& interval_start,
+                            int& events_in_interval,
+                            int max_events_per_interval) {
+  if (interval_start.is_null() || now < interval_start ||
+      now - interval_start >= kAggregateRateLimitInterval) {
+    interval_start = now;
+    events_in_interval = 0;
+  }
+  if (events_in_interval >= max_events_per_interval) {
+    return true;
+  }
+  ++events_in_interval;
+  return false;
+}
+
 }  // namespace
 
 PasswordProtectionJavaScriptFeature::PasswordProtectionJavaScriptFeature()
@@ -89,14 +118,17 @@
   if (!text || text->empty()) {
     return;
   }
-
   if (*event_type == kKeyDownEventType) {
     // A key event should consist of a single character. A longer string
-    // means the message isn't well-formed, so might be coming from a
-    // compromised WebProcess.
+    // means the message is not well-formed.
     if (base::CountUnicodeCharacters(*text) != 1) {
       return;
     }
+
+    if (IsKeyDownRateLimited(web_state)) {
+      return;
+    }
+
     observer->OnKeyPressed(*text);
   } else if (*event_type == kPasteEventType) {
     auto timer_it = paste_key_timers_.find(web_state);
@@ -134,6 +166,28 @@
   return false;
 }
 
+bool PasswordProtectionJavaScriptFeature::IsKeyDownRateLimited(
+    web::WebState* web_state) {
+  const base::TimeTicks now = base::TimeTicks::Now();
+  auto it = last_keydown_timestamps_.find(web_state);
+  if (it != last_keydown_timestamps_.end()) {
+    const base::TimeDelta elapsed = now - it->second;
+    if (elapsed < kKeyDownRateLimit) {
+      return true;
+    }
+  }
+
+  // Enforce the process-wide aggregate budget across all WebStates.
+  if (IsAggregateRateLimited(now, keydown_interval_start_,
+                             keydown_events_in_interval_,
+                             kMaxKeyDownEventsPerInterval)) {
+    return true;
+  }
+
+  last_keydown_timestamps_[web_state] = now;
+  return false;
+}
+
 void PasswordProtectionJavaScriptFeature::StartPasteKeyTimer(
     web::WebState* web_state) {
   auto& timer = paste_key_timers_[web_state];
@@ -177,4 +231,9 @@
   lookup_by_observer_.erase(observer);
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature_unittest.mm b/ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature_unittest.mm
index 7591e702..1d761184 100644
--- a/ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature_unittest.mm
+++ b/ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature_unittest.mm
@@ -4,6 +4,9 @@
 
 #import "ios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.h"
 
+#import <memory>
+#import <vector>
+
 #import "base/test/scoped_feature_list.h"
 #import "base/time/time.h"
 #import "base/values.h"
@@ -24,6 +27,7 @@
   virtual ~MockInputEventObserver() = default;
   void OnKeyPressed(std::string text) override {
     on_key_pressed_called_ = true;
+    key_pressed_count_++;
   }
   void OnPaste(std::string text) override {
     on_paste_called_ = true;
@@ -33,6 +37,7 @@
   web::WebState* web_state() const override { return web_state_; }
 
   bool on_key_pressed_called_ = false;
+  int key_pressed_count_ = 0;
   bool on_paste_called_ = false;
   bool on_paste_key_detected_called_ = false;
   std::string pasted_text_;
@@ -130,6 +135,9 @@
   EXPECT_TRUE(observer_->on_key_pressed_called_);
   observer_->on_key_pressed_called_ = false;
 
+  // Advance time by keydown rate limit.
+  task_environment_.FastForwardBy(base::Milliseconds(25));
+
   // A single supplementary Unicode code point (e.g., U+1F600 Grinning Face
   // emoji). It takes 2 UTF-16 code units (surrogate pair) but is 1 Unicode code
   // point.
@@ -144,6 +152,9 @@
   EXPECT_TRUE(observer_->on_key_pressed_called_);
   observer_->on_key_pressed_called_ = false;
 
+  // Advance time by keydown rate limit.
+  task_environment_.FastForwardBy(base::Milliseconds(25));
+
   // Multiple characters should be dropped.
   base::Value body3(
       base::DictValue().Set("eventType", "KeyDown").Set("text", "ab"));
@@ -252,6 +263,43 @@
   EXPECT_FALSE(observer_->on_paste_key_detected_called_);
 }
 
+// Tests that key down events are rate limited.
+TEST_F(PasswordProtectionJavaScriptFeatureTest, KeyDownEventRateLimited) {
+  base::Value body1(
+      base::DictValue().Set("eventType", "KeyDown").Set("text", "a"));
+  web::ScriptMessage message1(std::make_unique<base::Value>(std::move(body1)),
+                              /*is_user_interacting=*/true,
+                              /*is_main_frame=*/true,
+                              /*request_url=*/std::nullopt, url::Origin());
+
+  // First key down should be allowed.
+  feature_->ScriptMessageReceived(&web_state_, message1);
+  EXPECT_TRUE(observer_->on_key_pressed_called_);
+  observer_->on_key_pressed_called_ = false;
+
+  // Second key down immediately after should be dropped.
+  base::Value body2(
+      base::DictValue().Set("eventType", "KeyDown").Set("text", "b"));
+  web::ScriptMessage message2(std::make_unique<base::Value>(std::move(body2)),
+                              /*is_user_interacting=*/true,
+                              /*is_main_frame=*/true,
+                              /*request_url=*/std::nullopt, url::Origin());
+
+  feature_->ScriptMessageReceived(&web_state_, message2);
+  EXPECT_FALSE(observer_->on_key_pressed_called_);
+
+  // Third key down should be allowed after a sufficient amount of time.
+  base::Value body3(
+      base::DictValue().Set("eventType", "KeyDown").Set("text", "b"));
+  web::ScriptMessage message3(std::make_unique<base::Value>(std::move(body3)),
+                              /*is_user_interacting=*/true,
+                              /*is_main_frame=*/true,
+                              /*request_url=*/std::nullopt, url::Origin());
+  task_environment_.FastForwardBy(base::Milliseconds(25));
+  feature_->ScriptMessageReceived(&web_state_, message3);
+  EXPECT_TRUE(observer_->on_key_pressed_called_);
+}
+
 // Tests that if the text pasted event arrives after the coalescing window
 // has expired (and UIPasteboard has already been read), the text pasted event
 // is ignored (rate limited).
@@ -307,4 +355,90 @@
   EXPECT_FALSE(observer_->on_paste_key_detected_called_);
 }
 
+// Tests that keydown events across multiple WebStates are rate-limited in
+// aggregate.
+TEST_F(PasswordProtectionJavaScriptFeatureTest,
+       KeyDownAggregateRateLimitedAcrossMultipleWebStates) {
+  constexpr int kNumExtraWebStates = 4;
+  std::vector<std::unique_ptr<web::FakeWebState>> extra_web_states;
+  std::vector<std::unique_ptr<MockInputEventObserver>> extra_observers;
+  std::vector<web::WebState*> all_web_states;
+  std::vector<MockInputEventObserver*> all_observers;
+
+  all_web_states.push_back(&web_state_);
+  all_observers.push_back(observer_.get());
+
+  for (int i = 0; i < kNumExtraWebStates; ++i) {
+    auto ws = std::make_unique<web::FakeWebState>();
+    auto obs = std::make_unique<MockInputEventObserver>(ws.get());
+    feature_->AddObserver(obs.get());
+    all_web_states.push_back(ws.get());
+    all_observers.push_back(obs.get());
+    extra_web_states.push_back(std::move(ws));
+    extra_observers.push_back(std::move(obs));
+  }
+
+  // Send 80 keydown events round-robin across all WebStates.
+  // With 5 WebStates and 10ms between events, each WebState receives an event
+  // every 50ms (>= 25ms per-WebState limit), and 80 events * 10ms = 800ms total
+  // (< 1s aggregate window limit).
+  for (int i = 0; i < 80; ++i) {
+    web::WebState* target_ws = all_web_states[i % all_web_states.size()];
+    base::Value body(
+        base::DictValue().Set("eventType", "KeyDown").Set("text", "a"));
+    web::ScriptMessage message(std::make_unique<base::Value>(std::move(body)),
+                               /*is_user_interacting=*/true,
+                               /*is_main_frame=*/true,
+                               /*request_url=*/std::nullopt, url::Origin());
+    feature_->ScriptMessageReceived(target_ws, message);
+    task_environment_.FastForwardBy(base::Milliseconds(10));
+  }
+
+  int total_key_presses = 0;
+  for (auto* obs : all_observers) {
+    total_key_presses += obs->key_pressed_count_;
+  }
+  EXPECT_EQ(total_key_presses, 80);
+
+  // The 81st event within the same 1-second window should be dropped by the
+  // aggregate rate limit.
+  base::Value body_extra(
+      base::DictValue().Set("eventType", "KeyDown").Set("text", "b"));
+  web::ScriptMessage message_extra(
+      std::make_unique<base::Value>(std::move(body_extra)),
+      /*is_user_interacting=*/true,
+      /*is_main_frame=*/true,
+      /*request_url=*/std::nullopt, url::Origin());
+  feature_->ScriptMessageReceived(all_web_states[0], message_extra);
+
+  total_key_presses = 0;
+  for (auto* obs : all_observers) {
+    total_key_presses += obs->key_pressed_count_;
+  }
+  EXPECT_EQ(total_key_presses, 80);
+
+  // Fast forward by 1 second so the aggregate rate limit window resets.
+  task_environment_.FastForwardBy(base::Seconds(1));
+
+  // The next keydown should now be allowed.
+  base::Value body_after(
+      base::DictValue().Set("eventType", "KeyDown").Set("text", "c"));
+  web::ScriptMessage message_after(
+      std::make_unique<base::Value>(std::move(body_after)),
+      /*is_user_interacting=*/true,
+      /*is_main_frame=*/true,
+      /*request_url=*/std::nullopt, url::Origin());
+  feature_->ScriptMessageReceived(all_web_states[0], message_after);
+
+  total_key_presses = 0;
+  for (auto* obs : all_observers) {
+    total_key_presses += obs->key_pressed_count_;
+  }
+  EXPECT_EQ(total_key_presses, 81);
+
+  for (auto& obs : extra_observers) {
+    feature_->RemoveObserver(obs.get());
+  }
+}
+
 }  // namespace
Loading diff…

Original Bug Report

The reporter's bug is still restricted on the tracker. Chrome de-restricts security bugs ~30–90 days after the fix ships; a later run will backfill it here.