Chrome · Chrome for iOS
CVE-2026-17830
Logic Error in Chrome for iOS
Overview
Medium
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
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 <[email protected]> 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 <[email protected]> Commit-Queue: Olivier Robin <[email protected]> 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) {
Loading diff…
Regression Test / PoC
shipped with the fix
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)
Loading diff…
Original Bug Report
reported by [email protected]
Potential chrome:// navigation restriction bypass on iOS via Reading List and rel=opener
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: A potential security bypass in iOS Reading List navigation logic allows an attacker-controlled website to navigate a user’s offline Reading List tab to privileged chrome:// WebUI pages. When a user opens a distilled offline page under ‘chrome://offline’ that contains a link preserving ‘rel=opener’, the child tab retains a ‘window.opener’ reference. This reference allows the child page to cross-origin navigate the parent tab, which the browser permits because it erroneously treats the ‘chrome://offline’ base URL of the parent tab as a privileged app-specific context.
Affected files:
components/dom_distiller/core/javascript/content_processing.jsios/web/navigation/crw_wk_navigation_handler.mmios/chrome/browser/reading_list/model/offline_page_tab_helper.mm
Estimated timestamp from git blame: 2025-12-23
Description
A potential vulnerability in the link sanitization and navigation verification logic for iOS Reading List allows an attacker-controlled website to navigate a user’s offline Reading List tab to any arbitrary privileged local chrome:// WebUI page (such as chrome://flags or chrome://autofill-internals).
Note: The following findings and steps are based on static analysis of the source code. Our tooling agent does not currently have the ability to execute code or run a live proof of concept, so these should be treated as potential paths and behaviors.
This potential issue stems from two interacting defects:
-
Potential failure to strip or sanitize
relattributes during DOM distillation: Inthird_party/dom_distiller_js/dist/js/domdistiller.js, the attribute allowlist used during GWT sanitization contains the"rel"attribute. During post-extraction link sanitization insidecomponents/dom_distiller/core/javascript/content_processing.js, thesanitizeLinksfunction processes links in distilled pages. While it forces the target to_blank(linkElement.target = '_blank'), it does not modify or strip the existingrelattribute. As a result, if an article contains a link withrel="opener", this attribute survives the distillation process and is saved in the local offline HTML archive. -
Insecure verification of renderer-initiated
chrome://navigations on iOS: When a distilled Reading List article is loaded offline, the page is loaded into aWKWebViewwith the base URLchrome://offline?reload=<entry>. The navigation handler (ios/web/navigation/crw_wk_navigation_handler.mm) restricts renderer-initiated navigations to app-specific schemes usingshouldAllowAppSpecificURLNavigationAction:transition:. However, it permits navigations to app-specific URLs if the last committed URL of the web state is itself app-specific:web::NavigationItem* lastItem = self.webStateImpl->GetNavigationManager()->GetLastCommittedItem(); if (lastItem && (web::GetWebClient()->IsAppSpecificURL(lastItem->GetVirtualURL()) || web::GetWebClient()->IsAppSpecificURL(lastItem->GetURL()))) { // Last committed page is also app specific and navigation should be allowed. return YES; }Since the offline page is loaded with
chrome://offline?reload=...as its actual committed URL (lastItem->GetURL()),IsAppSpecificURLreturns true.
Potential Attack Scenario
- An attacker serves an article containing an anchor tag like:
<a href="https://attacker.example/payload" rel="opener">Read more</a> - The user adds this article to their Reading List. During distillation, DOM Distiller keeps the link and leaves the
rel="opener"attribute intact. - When the user opens the saved article offline or on a slow connection, it is loaded under the
chrome://offlineURL. The parent tab hasGetURL() == chrome://offline...andGetVirtualURL() == https://attacker.example/.... - The user taps the link, opening
https://attacker.example/payloadin a new tab. Becauserel="opener"is present, the browser preserves thewindow.openerreference pointing to the parent tab. - The loaded attacker page executes cross-origin location assignment to target the parent window:
window.opener.location = 'chrome://flags'; - The parent tab’s navigation handler detects that the parent window’s actual last committed URL is
chrome://offline, deems it app-specific, and allows the navigation tochrome://flagsto proceed.
Impact
An attacker can force the victim’s offline tab (which displays the legitimate-looking article URL in the omnibox) to navigate to any privileged chrome:// WebUI page. This bypasses the protections against renderer-initiated WebUI navigation on iOS, potentially exposing sensitive browser settings or administrative panels.
Suggested Fix
To resolve this potential issue, the following remediations should be considered:
- Sanitize
relattributes during distillation: Update DOM Distiller’s link sanitization code incomponents/dom_distiller/core/javascript/content_processing.jsto strip therelattribute or explicitly enforcerel="noopener"on any cross-origin links. - Restrict Navigation Privileges for Non-WebUI Pages: Refactor
shouldAllowAppSpecificURLNavigationAction:transition:inios/web/navigation/crw_wk_navigation_handler.mmso that pages loaded with non-privileged app-specific base URLs likechrome://offlineare not granted blanket permission to navigate to other privileged app-specific URLs. This can be achieved by only checkinglastItem->GetVirtualURL()or maintaining a list of strictly privileged WebUI pages rather than trusting any URL with thechrome://scheme.
Evaluated with Chrome root at commit: 5133b93d189b383c37805b1cf3a9d2dbfe8d7379
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.
References
On This Page