CVE-2026-17849
Overview
Changed Functions
| Function | Change | Notes |
|---|---|---|
ifios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios.mm |
modified | |
TEST_Fios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios_unittest.mm |
modified |
Files Changed
ios/chrome/app/profile/profile_controller.mmios/chrome/app/strings/ios_strings.grdios/chrome/browser/browser_view/ui_bundled/BUILD.gnios/chrome/browser/location_bar/ui_bundled/DEPSios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios.hios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios.mmios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios_unittest.mm
Patch
From 0ef746da2a08dea5bfe5719ad07a106e20630357 Mon Sep 17 00:00:00 2001
From: Olivier Robin <olivierrobin@google.com>
Date: Thu, 11 Jun 2026 05:13:52 -0700
Subject: [PATCH] Remove RL offline pages
Offline pages is not broadly used and concentrates a high share of
security issues. Fixing them would require a full rewriting of the
feature that low usage cannot justify.
There is also a partial overlap between Reading mode and Offline pages.
As a consequence it was decided to sunset the feature.
Fixed: 522233804, 518812672, 518051499, 518089997, 517710397, 517631680, 517690521, 517184957, 514510853, 514147906, 513507830
Change-Id: Ifdbddbc1d84e43bf162e37c792e73e308895fb79
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7904425
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1645282}
---
diff --git a/ios/chrome/app/profile/profile_controller.mm b/ios/chrome/app/profile/profile_controller.mm
index 35ce43e..eec857e 100644
--- a/ios/chrome/app/profile/profile_controller.mm
+++ b/ios/chrome/app/profile/profile_controller.mm
@@ -68,8 +68,6 @@
#import "ios/chrome/browser/mailto_handler/model/mailto_handler_service_factory.h"
#import "ios/chrome/browser/ntp/model/home_background_customization_promo_profile_agent.h"
#import "ios/chrome/browser/profile_metrics/model/profile_activity_profile_agent.h"
-#import "ios/chrome/browser/reading_list/model/reading_list_download_service.h"
-#import "ios/chrome/browser/reading_list/model/reading_list_download_service_factory.h"
#import "ios/chrome/browser/search_engines/model/extension_search_engine_data_updater.h"
#import "ios/chrome/browser/search_engines/model/search_engines_util.h"
#import "ios/chrome/browser/search_engines/model/template_url_service_factory.h"
@@ -125,9 +123,9 @@
NSString* const kStartupCreateMailtoHandlerService =
@"StartupCreateMailtoHandlerService";
-// Name of the block initializing the ReadingListDownloadService instance.
-NSString* const kStartupInitReadingListDownloadService =
- @"StartupInitReadingListDownloadService";
+// Name of the block cleaning up the offline reading list directory.
+NSString* const kStartupCleanupReadingListOfflineData =
+ @"StartupCleanupReadingListOfflineData";
// Name of the block that resynchronize the Spotlight index.
NSString* const kStartResyncSpotlightIndex = @"StartResyncSpotlightIndex";
@@ -745,7 +743,7 @@
[self scheduleClearingSessionCookies];
[self scheduleCleanupSessionStateCache];
[self scheduleCreateMailtoHandlerService];
- [self scheduleInitializeReadingListDownloadService];
+ [self scheduleCleanupReadingListOfflineData];
[self scheduleResyncSpotlightIndex];
[self scheduleCleanupFavicons];
[self scheduleLogStorageMetrics];
@@ -833,15 +831,15 @@
}];
}
-// Schedules initialization of the ReadingList download service.
-- (void)scheduleInitializeReadingListDownloadService {
+// Schedules cleanup of the ReadingList offline data directory.
+// TODO(crbug.com/522229299): Remove after Jun 2027.
+- (void)scheduleCleanupReadingListOfflineData {
DCHECK(_state.deferredRunner);
__weak ProfileController* weakSelf = self;
- [_state.deferredRunner
- enqueueBlockNamed:kStartupInitReadingListDownloadService
- block:^{
- [weakSelf initializeReadingListDownloadService];
- }];
+ [_state.deferredRunner enqueueBlockNamed:kStartupCleanupReadingListOfflineData
+ block:^{
+ [weakSelf cleanupReadingListOfflineData];
+ }];
}
// Schedules resynchronisation of the Spotlight index.
@@ -904,11 +902,18 @@
std::ignore = MailtoHandlerServiceFactory::GetForProfile(_state.profile);
}
-// Initializes the ReadingListDownloadService.
-- (void)initializeReadingListDownloadService {
+// Cleans up the ReadingList offline data directory.
+- (void)cleanupReadingListOfflineData {
DCHECK(_state.profile);
- ReadingListDownloadServiceFactory::GetForProfile(_state.profile)
- ->Initialize();
+ ProfileIOS* profile = _state.profile;
+ base::FilePath offline_directory =
+ profile->GetStatePath().Append(FILE_PATH_LITERAL("Offline"));
+ base::ThreadPool::PostTask(
+ FROM_HERE,
+ {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
+ base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN},
+ base::BindOnce(base::IgnoreResult(&base::DeletePathRecursively),
+ offline_directory));
}
// Resynchronizes the spotlight index.
diff --git a/ios/chrome/app/strings/ios_strings.grd b/ios/chrome/app/strings/ios_strings.grd
index 555038eb..2b7086b 100644
--- a/ios/chrome/app/strings/ios_strings.grd
+++ b/ios/chrome/app/strings/ios_strings.grd
@@ -5255,12 +5255,6 @@
<message name="IDS_IOS_PAGE_INFO_CONNECTION" desc="Title of the button opening the connection information (in page info)">
Connection
</message>
- <message name="IDS_IOS_PAGE_INFO_OFFLINE_PAGE" desc="Message to display in the page info when the page you are on is an offline page.">
- This page has been saved to your Reading List.
- </message>
- <message name="IDS_IOS_PAGE_INFO_OFFLINE_PAGE_LABEL" desc="Label to display at the top of the site information screen when the page you are on is an offline page.">
- Offline page
- </message>
<message name="IDS_IOS_PAGE_INFO_OFFLINE_TITLE" desc="Title of the message to display in the page info bubble when the page you are on is an offline page.">
Viewing offline version
</message>
diff --git a/ios/chrome/browser/browser_view/ui_bundled/BUILD.gn b/ios/chrome/browser/browser_view/ui_bundled/BUILD.gn
index bed08863..6a446735 100644
--- a/ios/chrome/browser/browser_view/ui_bundled/BUILD.gn
+++ b/ios/chrome/browser/browser_view/ui_bundled/BUILD.gn
@@ -438,6 +438,7 @@
"//ios/chrome/browser/commerce/model:shopping_service",
"//ios/chrome/browser/content_settings/model",
"//ios/chrome/browser/discover_feed/model:discover_feed_visibility_browser_agent",
+ "//ios/chrome/browser/dom_distiller/model",
"//ios/chrome/browser/download/model",
"//ios/chrome/browser/download/ui",
"//ios/chrome/browser/favicon/model",
diff --git a/ios/chrome/browser/location_bar/ui_bundled/DEPS b/ios/chrome/browser/location_bar/ui_bundled/DEPS
index 0fba61e3..feab4cc2 100644
--- a/ios/chrome/browser/location_bar/ui_bundled/DEPS
+++ b/ios/chrome/browser/location_bar/ui_bundled/DEPS
@@ -41,7 +41,6 @@
"+ios/chrome/browser/reader_mode/model/features.h",
"+ios/chrome/browser/reader_mode/model/reader_mode_web_state_utils.h",
"+ios/chrome/browser/reader_mode/ui/reader_mode_chip_visibility_delegate.h",
- "+ios/chrome/browser/reading_list/model/offline_page_tab_helper.h",
"+ios/chrome/browser/search_engines/model",
"+ios/chrome/browser/sharing/ui_bundled",
"+ios/chrome/browser/sync/model/send_tab_to_self_sync_service_factory.h",
diff --git a/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios.h b/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios.h
index 22a51e0..7ef7eb4 100644
--- a/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios.h
+++ b/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios.h
@@ -52,7 +52,6 @@
GetVisibleSecurityState() const override;
scoped_refptr<net::X509Certificate> GetCertificate() const override;
const gfx::VectorIcon* GetVectorIconOverride() const override;
- bool IsOfflinePage() const override;
bool IsNewTabPage() const override;
bool IsNewTabPageURL(const GURL& url) const override;
bool IsHomePage(const GURL& url) const override;
diff --git a/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios.mm b/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios.mm
index 13b1e04..72c8232a 100644
--- a/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios.mm
+++ b/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios.mm
@@ -13,7 +13,6 @@
#import "components/prefs/pref_service.h"
#import "components/security_state/ios/security_state_utils.h"
#import "ios/chrome/browser/autocomplete/model/autocomplete_scheme_classifier_impl.h"
-#import "ios/chrome/browser/reading_list/model/offline_page_tab_helper.h"
#import "ios/chrome/browser/search_engines/model/template_url_service_factory.h"
#import "ios/chrome/browser/shared/model/prefs/pref_names.h"
#import "ios/chrome/browser/shared/model/profile/profile_ios.h"
@@ -120,15 +119,6 @@
return nullptr;
}
-bool LocationBarModelDelegateIOS::IsOfflinePage() const {
- web::WebState* web_state = GetActiveWebState();
- if (!web_state) {
- return false;
- }
- OfflinePageTabHelper* helper = OfflinePageTabHelper::FromWebState(web_state);
- return helper && helper->presenting_offline_page();
-}
-
bool LocationBarModelDelegateIOS::IsNewTabPage() const {
// This is currently only called by the OmniboxEditModel to determine if the
// Google landing page is showing.
diff --git a/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios_unittest.mm b/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios_unittest.mm
index ed2b9c9..e4ec8149 100644
--- a/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios_unittest.mm
+++ b/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios_unittest.mm
@@ -37,19 +37,7 @@
std::unique_ptr<LocationBarModelDelegateIOS> delegate_;
};
-// Tests that IsOfflinePage() returns false if there is no WebState.
-TEST_F(LocationBarModelDelegateIOSTest, IsOfflinePage_NoWebState) {
- provider_.webState = nullptr;
- EXPECT_FALSE(delegate_->IsOfflinePage());
-}
-
-// Tests that IsOfflinePage() returns false if the WebState has no
-// OfflinePageTabHelper attached (to verify defensive null check for
-// b/505753157).
-TEST_F(LocationBarModelDelegateIOSTest, IsOfflinePage_NoHelper) {
Regression Test / PoC
diff --git a/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios_unittest.mm b/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios_unittest.mm
index ed2b9c9..e4ec8149 100644
--- a/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios_unittest.mm
+++ b/ios/chrome/browser/location_bar/ui_bundled/location_bar_model_delegate_ios_unittest.mm
@@ -37,19 +37,7 @@
std::unique_ptr<LocationBarModelDelegateIOS> delegate_;
};
-// Tests that IsOfflinePage() returns false if there is no WebState.
-TEST_F(LocationBarModelDelegateIOSTest, IsOfflinePage_NoWebState) {
- provider_.webState = nullptr;
- EXPECT_FALSE(delegate_->IsOfflinePage());
-}
-
-// Tests that IsOfflinePage() returns false if the WebState has no
-// OfflinePageTabHelper attached (to verify defensive null check for
-// b/505753157).
-TEST_F(LocationBarModelDelegateIOSTest, IsOfflinePage_NoHelper) {
- web::FakeWebState web_state;
- provider_.webState = &web_state;
- // Without OfflinePageTabHelper attached, this should return false and not
- // crash.
+// Tests that IsOfflinePage() returns false.
+TEST_F(LocationBarModelDelegateIOSTest, IsOfflinePage) {
EXPECT_FALSE(delegate_->IsOfflinePage());
}
diff --git a/ios/chrome/browser/reading_list/model/offline_page_tab_helper_unittest.mm b/ios/chrome/browser/reading_list/model/offline_page_tab_helper_unittest.mm
deleted file mode 100644
index e7287a8..0000000
--- a/ios/chrome/browser/reading_list/model/offline_page_tab_helper_unittest.mm
+++ /dev/null
@@ -1,286 +0,0 @@
-// Copyright 2019 The Chromium Authors
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#import "ios/chrome/browser/reading_list/model/offline_page_tab_helper.h"
-
-#import <memory>
-#import <vector>
-
-#import "base/memory/scoped_refptr.h"
-#import "base/run_loop.h"
-#import "base/test/ios/wait_util.h"
-#import "base/time/default_clock.h"
-#import "components/reading_list/core/fake_reading_list_model_storage.h"
-#import "components/reading_list/core/reading_list_entry.h"
-#import "components/reading_list/core/reading_list_model_impl.h"
-#import "components/sync/base/storage_type.h"
-#import "components/sync/model/wipe_model_upon_sync_disabled_behavior.h"
-#import "ios/chrome/browser/reading_list/model/reading_list_model_factory.h"
-#import "ios/chrome/browser/reading_list/model/reading_list_test_utils.h"
-#import "ios/chrome/browser/shared/model/profile/test/test_profile_ios.h"
-#import "ios/web/public/test/fakes/fake_navigation_context.h"
-#import "ios/web/public/test/fakes/fake_navigation_manager.h"
-#import "ios/web/public/test/fakes/fake_web_state.h"
-#import "ios/web/public/test/web_task_environment.h"
-#import "testing/gtest/include/gtest/gtest.h"
-#import "testing/platform_test.h"
-
-namespace {
-const char kTestURL[] = "http://foo.test";
-const char kTestSecondURL[] = "http://bar.test";
-const char kTestTitle[] = "title";
-const char kTestDistilledPath[] = "distilled.html";
-const char kTestDistilledURL[] = "http://foo.bar/distilled";
-} // namespace
-
-// Test fixture to test loading of Reading list offline pages.
-class OfflinePageTabHelperTest : public PlatformTest {
- public:
- void SetUp() override {
- PlatformTest::SetUp();
-
- std::vector<scoped_refptr<ReadingListEntry>> initial_entries;
- initial_entries.push_back(base::MakeRefCounted<ReadingListEntry>(
- GURL(kTestURL), kTestTitle, base::Time::Now()));
-
- TestProfileIOS::Builder builder;
- builder.AddTestingFactory(ReadingListModelFactory::GetInstance(),
- ReadingListModelTestingFactoryWithFakeStorage(
- std::move(initial_entries)));
- profile_ = std::move(builder).Build();
-
- fake_web_state_.SetBrowserState(profile_.get());
- fake_web_state_.SetNavigationManager(
- std::make_unique<web::FakeNavigationManager>());
-
- OfflinePageTabHelper::CreateForWebState(&fake_web_state_,
- reading_list_model());
- }
-
- ReadingListModel* reading_list_model() {
- return ReadingListModelFactory::GetForProfile(profile_.get());
- }
-
- protected:
- web::WebTaskEnvironment task_environment_;
- std::unique_ptr<TestProfileIOS> profile_;
- web::FakeWebState fake_web_state_;
-};
-
-// Test fixture to test loading of Reading list offline pages with a delayed
-// ReadingListModel.
-class OfflinePageTabHelperDelayedModelTest : public PlatformTest {
- public:
- void SetUp() override {
- PlatformTest::SetUp();
-
- auto storage = std::make_unique<FakeReadingListModelStorage>();
- fake_reading_list_model_storage_ = storage->AsWeakPtr();
-
- TestProfileIOS::Builder builder;
- builder.AddTestingFactory(
- ReadingListModelFactory::GetInstance(),
- base::BindRepeating(
- [](std::unique_ptr<FakeReadingListModelStorage>& storage,
- ProfileIOS* profile) -> std::unique_ptr<KeyedService> {
- DCHECK(storage.get());
- return std::make_unique<ReadingListModelImpl>(
- std::move(storage), syncer::StorageType::kUnspecified,
- syncer::WipeModelUponSyncDisabledBehavior::kNever,
- base::DefaultClock::GetInstance());
- },
- base::OwnedRef(std::move(storage))));
- profile_ = std::move(builder).Build();
-
- fake_web_state_.SetBrowserState(profile_.get());
- fake_web_state_.SetNavigationManager(
- std::make_unique<web::FakeNavigationManager>());
-
- OfflinePageTabHelper::CreateForWebState(&fake_web_state_,
- reading_list_model());
- }
-
- ReadingListModel* reading_list_model() {
- return ReadingListModelFactory::GetForProfile(profile_.get());
- }
-
- FakeReadingListModelStorage* fake_reading_list_model_storage() {
- return fake_reading_list_model_storage_.get();
- }
-
- protected:
- web::WebTaskEnvironment task_environment_;
- std::unique_ptr<TestProfileIOS> profile_;
- web::FakeWebState fake_web_state_;
- base::WeakPtr<FakeReadingListModelStorage> fake_reading_list_model_storage_;
-};
-
-// Tests that loading an online version does mark it read.
-TEST_F(OfflinePageTabHelperTest, TestLoadReadingListSuccess) {
- GURL url(kTestURL);
- scoped_refptr<const ReadingListEntry> entry =
- reading_list_model()->GetEntryByURL(url);
- fake_web_state_.SetCurrentURL(url);
- web::FakeNavigationContext context;
- context.SetUrl(url);
- context.SetHasCommitted(true);
- fake_web_state_.OnNavigationStarted(&context);
- fake_web_state_.OnNavigationFinished(&context);
- fake_web_state_.OnPageLoaded(web::PageLoadCompletionStatus::SUCCESS);
- EXPECT_FALSE(base::test::ios::WaitUntilConditionOrTimeout(
- base::test::ios::kWaitForFileOperationTimeout, ^bool {
- base::RunLoop().RunUntilIdle();
- return fake_web_state_.GetLastLoadedData();
- }));
- EXPECT_FALSE(fake_web_state_.GetLastLoadedData());
- EXPECT_TRUE(entry->IsRead());
- EXPECT_FALSE(OfflinePageTabHelper::FromWebState(&fake_web_state_)
- ->presenting_offline_page());
-}
-
-// Tests that failing loading an online version does not mark it read.
-TEST_F(OfflinePageTabHelperTest, TestLoadReadingListFailure) {
- GURL url(kTestURL);
- scoped_refptr<const ReadingListEntry> entry =
- reading_list_model()->GetEntryByURL(url);
- web::FakeNavigationContext context;
- context.SetUrl(url);
- context.SetHasCommitted(true);
- fake_web_state_.OnNavigationStarted(&context);
- fake_web_state_.OnNavigationFinished(&context);
- fake_web_state_.OnPageLoaded(web::PageLoadCompletionStatus::FAILURE);
- EXPECT_FALSE(base::test::ios::WaitUntilConditionOrTimeout(
- base::test::ios::kWaitForFileOperationTimeout, ^bool {
- base::RunLoop().RunUntilIdle();
- return fake_web_state_.GetLastLoadedData();
- }));
- EXPECT_FALSE(fake_web_state_.GetLastLoadedData());
- EXPECT_FALSE(entry->IsRead());
- EXPECT_FALSE(OfflinePageTabHelper::FromWebState(&fake_web_state_)
- ->presenting_offline_page());
-}
-
-// Tests that failing loading an online version will load the distilled version
-// and mark it read.
-TEST_F(OfflinePageTabHelperTest, TestLoadReadingListDistilled) {
- GURL url(kTestURL);
- std::string distilled_path = kTestDistilledPath;
- reading_list_model()->SetEntryDistilledInfoIfExists(
- url, base::FilePath(distilled_path), GURL(kTestDistilledURL), 50,
- base::Time::FromTimeT(100));
- scoped_refptr<const ReadingListEntry> entry =
- reading_list_model()->GetEntryByURL(url);
- fake_web_state_.SetCurrentURL(url);
- web::FakeNavigationContext context;
- context.SetHasCommitted(true);
- std::unique_ptr<web::NavigationItem> item = web::NavigationItem::Create();
- static_cast<web::FakeNavigationManager*>(
- fake_web_state_.GetNavigationManager())
- ->SetLastCommittedItem(item.get());
- context.SetUrl(url);
- fake_web_state_.OnNavigationStarted(&context);
- fake_web_state_.OnNavigationFinished(&context);
- fake_web_state_.OnPageLoaded(web::PageLoadCompletionStatus::FAILURE);
- EXPECT_FALSE(fake_web_state_.GetLastLoadedData());
- EXPECT_FALSE(entry->IsRead());
- EXPECT_TRUE(base::test::ios::WaitUntilConditionOrTimeout(
- base::test::ios::kWaitForFileOperationTimeout, ^bool {
- base::RunLoop().RunUntilIdle();
- return fake_web_state_.GetLastLoadedData();
- }));
- EXPECT_TRUE(entry->IsRead());
- EXPECT_TRUE(OfflinePageTabHelper::FromWebState(&fake_web_state_)
- ->presenting_offline_page());
-}
-
-// Tests that failing loading an online version does not load distilled
-// version if another navigation started.
-TEST_F(OfflinePageTabHelperTest, TestLoadReadingListFailureThenNavigate) {
- GURL url(kTestURL);
- GURL second_url(kTestSecondURL);
- scoped_refptr<const ReadingListEntry> entry =
- reading_list_model()->GetEntryByURL(url);
- web::FakeNavigationContext context;
- context.SetHasCommitted(true);
- context.SetUrl(url);
- fake_web_state_.OnNavigationStarted(&context);
- fake_web_state_.OnNavigationFinished(&context);
- fake_web_state_.OnPageLoaded(web::PageLoadCompletionStatus::FAILURE);
-
- web::FakeNavigationContext second_context;
- second_context.SetUrl(second_url);
- second_context.SetHasCommitted(true);
- fake_web_state_.OnNavigationStarted(&second_context);
- EXPECT_FALSE(base::test::ios::WaitUntilConditionOrTimeout(
- base::test::ios::kWaitForFileOperationTimeout, ^bool {
- base::RunLoop().RunUntilIdle();
- return fake_web_state_.GetLastLoadedData();
- }));
- EXPECT_FALSE(fake_web_state_.GetLastLoadedData());
- EXPECT_FALSE(entry->IsRead());
- EXPECT_FALSE(OfflinePageTabHelper::FromWebState(&fake_web_state_)
- ->presenting_offline_page());
-}
-
-// Tests that OfflinePageTabHelper correctly reports existence of a distilled
-// version.
-TEST_F(OfflinePageTabHelperTest, TestHasDistilledVersionForOnlineUrl) {
- OfflinePageTabHelper* offline_page_tab_helper =
- OfflinePageTabHelper::FromWebState(&fake_web_state_);
- GURL url(kTestURL);
- EXPECT_FALSE(offline_page_tab_helper->HasDistilledVersionForOnlineUrl(url));
- GURL second_url(kTestSecondURL);
- EXPECT_FALSE(
- offline_page_tab_helper->HasDistilledVersionForOnlineUrl(second_url));
-
- std::string distilled_path = kTestDistilledPath;
- reading_list_model()->SetEntryDistilledInfoIfExists(
- url, base::FilePath(distilled_path), GURL(kTestDistilledURL), 50,
- base::Time::FromTimeT(100));
- EXPECT_TRUE(offline_page_tab_helper->HasDistilledVersionForOnlineUrl(url));
-}
-
-// Tests that OfflinePageTabHelper correctly shows Offline page if model takes
-// a long time to load.
-TEST_F(OfflinePageTabHelperDelayedModelTest, TestLateReadingListModelLoading) {
- OfflinePageTabHelper* offline_page_tab_helper =
- OfflinePageTabHelper::FromWebState(&fake_web_state_);
- GURL url(kTestURL);
- EXPECT_FALSE(offline_page_tab_helper->HasDistilledVersionForOnlineUrl(url));
- web::FakeNavigationContext context;
-
- context.SetHasCommitted(true);
- std::unique_ptr<web::NavigationItem> item = web::NavigationItem::Create();
- static_cast<web::FakeNavigationManager*>(
- fake_web_state_.GetNavigationManager())
- ->SetLastCommittedItem(item.get());
- context.SetUrl(url);
- fake_web_state_.OnNavigationStarted(&context);
- fake_web_state_.OnNavigationFinished(&context);
- fake_web_state_.OnPageLoaded(web::PageLoadCompletionStatus::FAILURE);
- EXPECT_FALSE(base::test::ios::WaitUntilConditionOrTimeout(
- base::test::ios::kWaitForFileOperationTimeout, ^bool {
- base::RunLoop().RunUntilIdle();
- return fake_web_state_.GetLastLoadedData();
- }));
- EXPECT_FALSE(offline_page_tab_helper->presenting_offline_page());
- // Complete the reading list model load from storage.
... (truncated)
Original Bug Report
Potential security interstitial bypass via provisional preemption in OfflinePageTabHelper
Project Fortify, an experimental security project, has identified the following potential security issue. If you’re a feature owner CC-ed on this bug, please do your best to review these reports. Please see https://chromium.googlesource.com/chromium/src/+/main/docs/security/ai-generated-security-bugs-faq.md for more information.
Overview: In iOS Chrome, a 1.5-second slow-load progress timer in OfflinePageTabHelper can trigger an offline placeholder navigation while the target navigation is still provisional. This cancels the pending navigation before security checks (such as SSL, Safe Browsing, or HTTPS-Only Mode) can complete. As a result, security interstitials are bypassed, and the browser displays the locally-distilled cached page under the spoofed online URL in the omnibox.
Affected files:
ios/chrome/browser/reading_list/model/offline_page_tab_helper.mm
Estimated timestamp from git blame: 2019-03-11
Root Cause Analysis
In ios/chrome/browser/reading_list/model/offline_page_tab_helper.mm, a 1.5-second repeating timer is started upon beginning a navigation to check loading progress. If the loading progress (measured via [WKWebView estimatedProgress]) is slow (progress < 25% at 1.5s), OfflinePageTabHelper::CheckLoadingProgress triggers and calls PresentOfflinePageForOnlineUrl to swap in the offline distilled version.
At the top of PresentOfflinePageForOnlineUrl, there is a guard designed to prevent loading the offline page if a security interstitial is showing:
security_interstitials::IOSBlockingPageTabHelper* blocking_page_tab_helper =
security_interstitials::IOSBlockingPageTabHelper::FromWebState(web_state_);
if (blocking_page_tab_helper &&
blocking_page_tab_helper->GetCurrentBlockingPage()) {
return;
}
However, because the target navigation is still provisional and has not yet committed or failed, GetCurrentBlockingPage() (which only consults the committed blocking page in ios/components/security_interstitials/ios_blocking_page_tab_helper.mm) returns nullptr.
Consequently, the helper enters the placeholder load branch:
if (!is_offline_navigation_ && is_new_navigation_ && !navigation_committed_) {
GURL offlineURL = reading_list::OfflineURLForURL(entry_url);
web::NavigationManager::WebLoadParams params(offlineURL);
params.transition_type = navigation_transition_type_;
params.virtual_url = entry_url;
params.is_renderer_initiated = navigation_is_renderer_initiated_;
web_state_->GetNavigationManager()->LoadURLWithParams(params);
return;
}
This triggers a new load of the chrome://offline URL. In WKWebView, initiating this new load immediately cancels the in-progress provisional navigation to the target URL. This preempts the provisional navigation before security handlers (like Safe Browsing, SSL bad cert detection, or HTTPS-Only upgrade failures) can complete their checks or display their warning pages.
Once the placeholder navigation finishes (handled via PrepareErrorPage which suppresses the default error page and loads the distilled page data), LoadData commits the distilled HTML with the virtual URL set to the original target (e.g. https://victim.example). The omnibox displays the target URL, but the user is viewing the locally-cached distilled page with zero security warnings (no broken lock, no red warning, no upgrade fallback warning).
Potential Attack Scenario
Note: The following steps are potential/suggested because our automated analysis tools do not have the capability to run code or execute a live proof of concept in an interactive iOS environment.
- The victim has a Reading List entry for
https://victim.example/articlethat has been successfully distilled (ReadingListEntry::PROCESSED). - The victim initiates a navigation to
https://victim.example/article(e.g., via a link or Reading List row). - An attacker controlling the network path or target server intentionally holds the TLS handshake (e.g., stalling the
ServerHello) or stalls the HTTP response headers for at least 1.5 seconds. - Due to the stall, the WKWebView load progress remains under 25% for 1.5 seconds, triggering
CheckLoadingProgress. - The helper cancels the pending provisional navigation (preempting any SSL, Safe Browsing, or HTTPS-Only Mode warnings) and successfully loads the distilled document locally.
- The victim is shown the offline page with
https://victim.example/articlein the address bar, with no security warnings, masking the active threat.
Suggested Fix
To prevent this bypass, OfflinePageTabHelper should not preempt provisional navigations when a security check is in progress or when there is an active provisional blocking page.
Specifically, IOSBlockingPageTabHelper should be updated to track and expose whether there are any pending/provisional blocking pages associated with the active navigation. The guard in PresentOfflinePageForOnlineUrl can then check this state to avoid loading the offline page if a provisional blocking page is registered:
// Suggested modification
if (blocking_page_tab_helper &&
(blocking_page_tab_helper->GetCurrentBlockingPage() ||
blocking_page_tab_helper->HasPendingBlockingPage())) {
return;
}
Alternatively, ensure that the 1.5-second slow-load timer is paused or ignored if a security error or upgrade fallback handler is active on the pending navigation.
Evaluated with Chrome root at commit: 87214e6721f6c34afd9181b80769a24c0c601c50
Results so far have been promising, but there can be wrong deductions. Feel free to adjust as follows:
- If you are familiar with the severity guidelines, you may adjust the severity.
- If this is a false positive, and there’s no work to be done, please close as WAI.
- If there is work to do here but not a vulnerability, please change the issue type to Task/Bug/FR.
Data from false positives will be used to improve accuracy over time. And please feel free to reach out to me directly if you have concerns or feedback on the project.