Chrome · Safebrowsing
CVE-2026-87518
Logic Error in Safebrowsing
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
forios/chrome/browser/safe_browsing/model/password_protection_egtest.mm |
modified | |
ifios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.mm |
modified |
Files Changed
ios/chrome/browser/safe_browsing/model/password_protection_egtest.mmios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.hios/chrome/browser/safe_browsing/model/password_protection_java_script_feature.mm
Patch
From 74f96d1660aa390f1b30a8473f7f793a36966c57 Mon Sep 17 00:00:00 2001 From: Daniel White <[email protected]> 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 <[email protected]> > > Commit-Queue: Daniel White <[email protected]> > > 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 <[email protected]> > Reviewed-by: jdh <[email protected]> > 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 <[email protected]> Reviewed-by: jdh <[email protected]> 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.
References
On This Page