From bb617c8b09d43ed4d617d0c2b6769899324c9ee8 Mon Sep 17 00:00:00 2001
From: Justin Cohen <[email protected]>
Date: Fri, 21 Aug 2026 12:07:28 -0700
Subject: [PATCH] ios: Validate error page navigations in CRWWKNavigationHandler.

Error page navigations inside CRWWKNavigationHandler can currently be
initiated by any context. This change ensures that navigations to local
error page file URLs are only allowed if explicitly initiated by the
browser process, or if they are safe back/forward or reload navigations
of already committed pages.

It also allows error page loads during active session restoration,
which is securely managed by the system.

Bug: 508462481
Change-Id: Ic17c3eaa553b80ca5a62de4ab0178be38a5de8b4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7862880
Auto-Submit: Justin Cohen <[email protected]>
Reviewed-by: Matt Reichhoff <[email protected]>
Reviewed-by: Mike Dougherty <[email protected]>
Commit-Queue: Mike Dougherty <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1684115}
---
diff --git a/ios/web/navigation/crw_wk_navigation_handler.mm b/ios/web/navigation/crw_wk_navigation_handler.mm
index 2956871..716679ef 100644
--- a/ios/web/navigation/crw_wk_navigation_handler.mm
+++ b/ios/web/navigation/crw_wk_navigation_handler.mm
@@ -179,6 +179,7 @@
 }
 
 @property(nonatomic, weak) id<CRWWKNavigationHandlerDelegate> delegate;
+@property(nonatomic, copy) NSURL* allowedErrorPageFileURL;
 
 // Returns the WebStateImpl from self.delegate.
 @property(nonatomic, readonly, assign) web::WebStateImpl* webStateImpl;
@@ -331,9 +332,31 @@
     }
   }
 
-  // If this is a error navigation, pass through.
+  // Error pages must only be loaded in the main frame. Unconditionally clear
+  // the allowed error page URL on any main frame navigation to prevent dangling
+  // states (e.g., if a load HTML string request was made instead of a
+  // file URL load).
+  NSURL* allowedErrorPageFileURL = self.allowedErrorPageFileURL;
+  if (action.targetFrame.mainFrame) {
+    self.allowedErrorPageFileURL = nil;
+  }
+
+  // If this is an error navigation, pass through only if initiated by the
+  // browser, or if it is a back/forward or reload of an already committed page.
   if ([CRWErrorPageHelper isErrorPageFileURL:requestURL]) {
-    decisionHandler(WKNavigationActionPolicyAllow);
+    BOOL isHistoryOrReloadOrRestore =
+        action.navigationType == WKNavigationTypeBackForward ||
+        action.navigationType == WKNavigationTypeReload ||
+        self.navigationManagerImpl->IsRestoreSessionInProgress();
+    BOOL isExpectedErrorPage =
+        allowedErrorPageFileURL &&
+        requestURL == net::GURLWithNSURL(allowedErrorPageFileURL);
+    if ((isExpectedErrorPage && action.targetFrame.mainFrame) ||
+        isHistoryOrReloadOrRestore) {
+      decisionHandler(WKNavigationActionPolicyAllow);
+      return;
+    }
+    decisionHandler(WKNavigationActionPolicyCancel);
     return;
   }
 
@@ -1483,9 +1506,12 @@
     }
   }
 
-  // Allow navigation to WebUI pages from error pages.
+  // Allow navigation from an error page to the failed URL it represents so
+  // that error pages for app-specific URLs can retry the original navigation.
   if ([CRWErrorPageHelper isErrorPageFileURL:self.documentURL]) {
-    return YES;
+    return requestURL ==
+           [CRWErrorPageHelper
+               failedNavigationURLFromErrorPageFileURL:self.documentURL];
   }
 
   if (!action.sourceFrame.mainFrame) {
@@ -2144,6 +2170,7 @@
           isErrorPageFileURLForFailedNavigationURL:backForwardItem.URL] &&
       !isSameURLFromWebClient &&
       ![backForwardItem.URL isEqual:errorPage.failedNavigationURL]) {
+    self.allowedErrorPageFileURL = errorPage.errorPageFileURL;
     errorNavigation = [webView loadFileURL:errorPage.errorPageFileURL
                    allowingReadAccessToURL:errorPage.errorPageFileURL];
   } else {
diff --git a/ios/web/navigation/navigation_manager_impl.h b/ios/web/navigation/navigation_manager_impl.h
index d71cde27..0951dbe 100644
--- a/ios/web/navigation/navigation_manager_impl.h
+++ b/ios/web/navigation/navigation_manager_impl.h
@@ -208,6 +208,7 @@
   // TODO(crbug.com/41284081): Make this private once all navigation code is
   // moved out of CRWWebController.
   NavigationItemImpl* GetCurrentItemImpl() const;
+  bool IsRestoreSessionInProgress() const;
 
   // Implementation for corresponding NavigationManager getters.
   NavigationItemImpl* GetPendingItemImpl() const;
diff --git a/ios/web/navigation/navigation_manager_impl.mm b/ios/web/navigation/navigation_manager_impl.mm
index 1aa8b484..4719013 100644
--- a/ios/web/navigation/navigation_manager_impl.mm
+++ b/ios/web/navigation/navigation_manager_impl.mm
@@ -847,6 +847,10 @@
   return GetLastCommittedItemImpl();
 }
 
+bool NavigationManagerImpl::IsRestoreSessionInProgress() const {
+  return native_restore_in_progress_;
+}
+
 NavigationItemImpl* NavigationManagerImpl::GetLastCommittedItemImpl() const {
   if (empty_window_open_item_) {
     return empty_window_open_item_.get();
diff --git a/ios/web/web_state/ui/crw_web_controller_unittest.mm b/ios/web/web_state/ui/crw_web_controller_unittest.mm
index 0cc4a777..34f234f 100644
--- a/ios/web/web_state/ui/crw_web_controller_unittest.mm
+++ b/ios/web/web_state/ui/crw_web_controller_unittest.mm
@@ -9,6 +9,7 @@
 #import <memory>
 #import <utility>
 
+#import "base/apple/bundle_locations.h"
 #import "base/apple/foundation_util.h"
 #import "base/scoped_observation.h"
 #import "base/strings/sys_string_conversions.h"
@@ -22,6 +23,7 @@
 #import "ios/web/common/uikit_ui_util.h"
 #import "ios/web/js_messaging/web_view_js_utils.h"
 #import "ios/web/navigation/block_universal_links_buildflags.h"
+#import "ios/web/navigation/crw_wk_navigation_handler.h"
 #import "ios/web/navigation/crw_wk_navigation_states.h"
 #import "ios/web/navigation/navigation_item_impl.h"
 #import "ios/web/navigation/navigation_manager_impl.h"
@@ -62,6 +64,27 @@
 #import "third_party/ocmock/ocmock_extensions.h"
 #import "url/scheme_host_port.h"
 
+@interface CRWWebController (Testing)
+@property(nonatomic, readonly) CRWWKNavigationHandler* navigationHandler;
+@end
+
+@interface CRWWKNavigationHandler (Testing)
+@property(nonatomic, copy) NSURL* allowedErrorPageFileURL;
+@end
+@interface FakeWKFrameInfo : NSObject <NSCopying>
+@property(nonatomic, assign, getter=isMainFrame) BOOL mainFrame;
+@end
+
+@implementation FakeWKFrameInfo
+@synthesize mainFrame = _mainFrame;
+
+- (id)copyWithZone:(NSZone*)zone {
+  FakeWKFrameInfo* copy = [[[self class] allocWithZone:zone] init];
+  copy.mainFrame = self.mainFrame;
+  return copy;
+}
+@end
+
 using base::test::ios::kWaitForJSCompletionTimeout;
 using base::test::ios::kWaitForPageLoadTimeout;
 using base::test::ios::WaitUntilConditionOrTimeout;
@@ -1248,6 +1271,85 @@
       url_request, WKNavigationActionPolicyCancel));
 }
 
+// Tests that a forged navigation to an error page URL is blocked/cancelled
+// when the browser didn't initiate it, and successfully allowed when the
+// browser did, but restricted strictly to main frame navigations and cleared.
+TEST_F(CRWWebControllerPolicyDeciderTest, RejectForgedErrorPageNavigation) {
+  NSString* path =
+      [base::apple::FrameworkBundle() pathForResource:@"error_page_loaded"
+                                               ofType:@"html"];
+  ASSERT_TRUE(path);
+  NSURL* forged_error_url = [NSURL
+      URLWithString:[NSString
+                        stringWithFormat:@"file://%@?url=chrome://settings",
+                                         path]];
+  NSMutableURLRequest* forged_request =
+      [NSMutableURLRequest requestWithURL:forged_error_url];
+  forged_request.mainDocumentURL = forged_error_url;
+
+  // Use FakeWKFrameInfo to avoid OCMock copying and swizzling issues.
+  FakeWKFrameInfo* fake_frame = [[FakeWKFrameInfo alloc] init];
+
+  CRWFakeWKNavigationAction* action = [[CRWFakeWKNavigationAction alloc] init];
+  action.request = forged_request;
+
+  // Default case: error page load is blocked when not initiated by browser.
+  fake_frame.mainFrame = YES;
+  action.targetFrame = (WKFrameInfo*)fake_frame;
+  __block bool callback_called = false;
+  [navigation_delegate_ webView:mock_web_view_
+      decidePolicyForNavigationAction:action
+                          preferences:[[WKWebpagePreferences alloc] init]
+                      decisionHandler:^(WKNavigationActionPolicy policy,
+                                        WKWebpagePreferences* ignored) {
+                        EXPECT_EQ(policy, WKNavigationActionPolicyCancel);
+                        callback_called = true;
+                      }];
+  EXPECT_TRUE(WaitUntilConditionOrTimeout(kWaitForPageLoadTimeout, ^{
+    return callback_called;
+  }));
+
+  [web_controller() navigationHandler].allowedErrorPageFileURL =
+      forged_error_url;
+
+  // Subframe error page load is blocked and does not clear the flag.
+  fake_frame.mainFrame = NO;
+  action.targetFrame = (WKFrameInfo*)fake_frame;
+  callback_called = false;
+  [navigation_delegate_ webView:mock_web_view_
+      decidePolicyForNavigationAction:action
+                          preferences:[[WKWebpagePreferences alloc] init]
+                      decisionHandler:^(WKNavigationActionPolicy policy,
+                                        WKWebpagePreferences* ignored) {
+                        EXPECT_EQ(policy, WKNavigationActionPolicyCancel);
+                        callback_called = true;
+                      }];
+  EXPECT_TRUE(WaitUntilConditionOrTimeout(kWaitForPageLoadTimeout, ^{
+    return callback_called;
+  }));
+
+  EXPECT_TRUE([[web_controller() navigationHandler].allowedErrorPageFileURL
+      isEqual:forged_error_url]);
+
+  // Main frame navigation is allowed and clears the flag.
+  fake_frame.mainFrame = YES;
+  action.targetFrame = (WKFrameInfo*)fake_frame;
+  callback_called = false;
+  [navigation_delegate_ webView:mock_web_view_
+      decidePolicyForNavigationAction:action
+                          preferences:[[WKWebpagePreferences alloc] init]
+                      decisionHandler:^(WKNavigationActionPolicy policy,
+                                        WKWebpagePreferences* ignored) {
+                        EXPECT_EQ(policy, WKNavigationActionPolicyAllow);
+                        callback_called = true;
+                      }];
+  EXPECT_TRUE(WaitUntilConditionOrTimeout(kWaitForPageLoadTimeout, ^{
+    return callback_called;
+  }));
+
+  EXPECT_FALSE([web_controller() navigationHandler].allowedErrorPageFileURL);
+}
+
 // Test fixture for window.open tests.
 class WindowOpenByDomTest : public WebTestWithWebController {
  protected:

diff --git a/ios/web/web_state/ui/crw_web_controller_unittest.mm b/ios/web/web_state/ui/crw_web_controller_unittest.mm
index 0cc4a777..34f234f 100644
--- a/ios/web/web_state/ui/crw_web_controller_unittest.mm
+++ b/ios/web/web_state/ui/crw_web_controller_unittest.mm
@@ -9,6 +9,7 @@
 #import <memory>
 #import <utility>
 
+#import "base/apple/bundle_locations.h"
 #import "base/apple/foundation_util.h"
 #import "base/scoped_observation.h"
 #import "base/strings/sys_string_conversions.h"
@@ -22,6 +23,7 @@
 #import "ios/web/common/uikit_ui_util.h"
 #import "ios/web/js_messaging/web_view_js_utils.h"
 #import "ios/web/navigation/block_universal_links_buildflags.h"
+#import "ios/web/navigation/crw_wk_navigation_handler.h"
 #import "ios/web/navigation/crw_wk_navigation_states.h"
 #import "ios/web/navigation/navigation_item_impl.h"
 #import "ios/web/navigation/navigation_manager_impl.h"
@@ -62,6 +64,27 @@
 #import "third_party/ocmock/ocmock_extensions.h"
 #import "url/scheme_host_port.h"
 
+@interface CRWWebController (Testing)
+@property(nonatomic, readonly) CRWWKNavigationHandler* navigationHandler;
+@end
+
+@interface CRWWKNavigationHandler (Testing)
+@property(nonatomic, copy) NSURL* allowedErrorPageFileURL;
+@end
+@interface FakeWKFrameInfo : NSObject <NSCopying>
+@property(nonatomic, assign, getter=isMainFrame) BOOL mainFrame;
+@end
+
+@implementation FakeWKFrameInfo
+@synthesize mainFrame = _mainFrame;
+
+- (id)copyWithZone:(NSZone*)zone {
+  FakeWKFrameInfo* copy = [[[self class] allocWithZone:zone] init];
+  copy.mainFrame = self.mainFrame;
+  return copy;
+}
+@end
+
 using base::test::ios::kWaitForJSCompletionTimeout;
 using base::test::ios::kWaitForPageLoadTimeout;
 using base::test::ios::WaitUntilConditionOrTimeout;
@@ -1248,6 +1271,85 @@
       url_request, WKNavigationActionPolicyCancel));
 }
 
+// Tests that a forged navigation to an error page URL is blocked/cancelled
+// when the browser didn't initiate it, and successfully allowed when the
+// browser did, but restricted strictly to main frame navigations and cleared.
+TEST_F(CRWWebControllerPolicyDeciderTest, RejectForgedErrorPageNavigation) {
+  NSString* path =
+      [base::apple::FrameworkBundle() pathForResource:@"error_page_loaded"
+                                               ofType:@"html"];
+  ASSERT_TRUE(path);
+  NSURL* forged_error_url = [NSURL
+      URLWithString:[NSString
+                        stringWithFormat:@"file://%@?url=chrome://settings",
+                                         path]];
+  NSMutableURLRequest* forged_request =
+      [NSMutableURLRequest requestWithURL:forged_error_url];
+  forged_request.mainDocumentURL = forged_error_url;
+
+  // Use FakeWKFrameInfo to avoid OCMock copying and swizzling issues.
+  FakeWKFrameInfo* fake_frame = [[FakeWKFrameInfo alloc] init];
+
+  CRWFakeWKNavigationAction* action = [[CRWFakeWKNavigationAction alloc] init];
+  action.request = forged_request;
+
+  // Default case: error page load is blocked when not initiated by browser.
+  fake_frame.mainFrame = YES;
+  action.targetFrame = (WKFrameInfo*)fake_frame;
+  __block bool callback_called = false;
+  [navigation_delegate_ webView:mock_web_view_
+      decidePolicyForNavigationAction:action
+                          preferences:[[WKWebpagePreferences alloc] init]
+                      decisionHandler:^(WKNavigationActionPolicy policy,
+                                        WKWebpagePreferences* ignored) {
+                        EXPECT_EQ(policy, WKNavigationActionPolicyCancel);
+                        callback_called = true;
+                      }];
+  EXPECT_TRUE(WaitUntilConditionOrTimeout(kWaitForPageLoadTimeout, ^{
+    return callback_called;
+  }));
+
+  [web_controller() navigationHandler].allowedErrorPageFileURL =
+      forged_error_url;
+
+  // Subframe error page load is blocked and does not clear the flag.
+  fake_frame.mainFrame = NO;
+  action.targetFrame = (WKFrameInfo*)fake_frame;
+  callback_called = false;
+  [navigation_delegate_ webView:mock_web_view_
+      decidePolicyForNavigationAction:action
+                          preferences:[[WKWebpagePreferences alloc] init]
+                      decisionHandler:^(WKNavigationActionPolicy policy,
+                                        WKWebpagePreferences* ignored) {
+                        EXPECT_EQ(policy, WKNavigationActionPolicyCancel);
+                        callback_called = true;
+                      }];
+  EXPECT_TRUE(WaitUntilConditionOrTimeout(kWaitForPageLoadTimeout, ^{
+    return callback_called;
+  }));
+
+  EXPECT_TRUE([[web_controller() navigationHandler].allowedErrorPageFileURL
+      isEqual:forged_error_url]);
+
+  // Main frame navigation is allowed and clears the flag.
+  fake_frame.mainFrame = YES;
+  action.targetFrame = (WKFrameInfo*)fake_frame;
+  callback_called = false;
+  [navigation_delegate_ webView:mock_web_view_
+      decidePolicyForNavigationAction:action
+                          preferences:[[WKWebpagePreferences alloc] init]
+                      decisionHandler:^(WKNavigationActionPolicy policy,
+                                        WKWebpagePreferences* ignored) {
+                        EXPECT_EQ(policy, WKNavigationActionPolicyAllow);
+                        callback_called = true;
+                      }];
+  EXPECT_TRUE(WaitUntilConditionOrTimeout(kWaitForPageLoadTimeout, ^{
+    return callback_called;
+  }));
+
+  EXPECT_FALSE([web_controller() navigationHandler].allowedErrorPageFileURL);
+}
+
 // Test fixture for window.open tests.
 class WindowOpenByDomTest : public WebTestWithWebController {
  protected: