From 8b2b8460220a3dda1d92e49debf9a31e5ecc3261 Mon Sep 17 00:00:00 2001
From: Wolfgang Beyer <wolfi@chromium.org>
Date: Fri, 18 Sep 2026 06:32:34 -0700
Subject: [PATCH] [DevTools] Restrict Page history commands to accessible URLs

PageHandler::Navigate() checks the client's trust level and its local
file access permission before navigating, but
PageHandler::GetNavigationHistory() and
PageHandler::NavigateToHistoryEntry() do not. A client that may not
attach to file:// URLs, such as an extension using the debugger API
without "Allow access to file URLs", can therefore read the file://
URLs and titles in the attached tab's session history, and traverse
the tab back to one of them. Page.navigate(), chrome.tabs.update() and
chrome.tabs.get() all deny that same client the same information and
the same navigation.

Extract the checks from Navigate() into CheckNavigationAllowed() and
apply them to both history commands, so that a single policy governs
all three. A client that may not navigate the page to a URL must not
learn it either, so the same check gates disclosure:
GetNavigationHistory() redacts the url, userTypedURL and title of
entries that fail it, keeping the entries themselves so that
currentIndex and the entry ids still match the actual history, and
NavigateToHistoryEntry() rejects traversals to those entries.

TAG=agy
CONV=e9d27f4f-02a8-423d-abe6-a7a4531e4833

Bug: 514445657
Change-Id: I54a45030bfcaf5e78b0fc109c311dbf945bf720a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8425785
Auto-Submit: Wolfgang Beyer <wolfi@chromium.org>
Reviewed-by: Danil Somsikov <dsv@chromium.org>
Commit-Queue: Danil Somsikov <dsv@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1700708}
---
diff --git a/content/browser/devtools/protocol/page_handler.cc b/content/browser/devtools/protocol/page_handler.cc
index 4acbbae..9c0551d 100644
--- a/content/browser/devtools/protocol/page_handler.cc
+++ b/content/browser/devtools/protocol/page_handler.cc
@@ -930,6 +930,47 @@
 
 }  // namespace
 
+Response PageHandler::CheckNavigationAllowed(const GURL& url) {
+  CHECK(host_);
+
+  GURL inner_url = url;
+  if (url.SchemeIs(content::kViewSourceScheme)) {
+    inner_url = GURL(url.GetContent());
+  }
+
+  bool is_file = inner_url.SchemeIsFile();
+#if BUILDFLAG(IS_CHROMEOS)
+  // The "externalfile" scheme is ChromeOS-specific.
+  is_file |= inner_url.SchemeIs(content::kExternalFileScheme);
+#endif
+
+  if (is_file && !may_read_local_files_) {
+    return Response::ServerError("Navigating to local URL is not allowed");
+  }
+
+  // chrome-untrusted:// WebUIs might perform high-privileged actions on
+  // navigation, disallow navigation to them unless the client is trusted.
+  if ((inner_url.SchemeIs(kChromeUIUntrustedScheme) ||
+       inner_url.SchemeIs(kChromeDevToolsScheme)) &&
+      !is_trusted_) {
+    return Response::ServerError(
+        "Navigating to a URL with a privileged scheme is not allowed");
+  }
+
+  if (!session()->GetClient()->MayAttachToURL(url,
+                                              host_->web_ui() != nullptr)) {
+    url::Origin origin = url::Origin::Create(url);
+    if (!origin.scheme().empty() &&
+        origin.scheme() != content::kChromeUIScheme &&
+        origin.scheme() != content::kChromeUIUntrustedScheme &&
+        origin.scheme() != content::kChromeDevToolsScheme) {
+      return Response::ServerError("Not allowed");
+    }
+  }
+
+  return Response::Success();
+}
+
 void PageHandler::Navigate(const std::string& url,
                            std::optional<std::string> referrer,
                            std::optional<std::string> maybe_transition_type,
@@ -943,50 +984,17 @@
     return;
   }
 
-  GURL inner_url = gurl;
-  if (gurl.SchemeIs(content::kViewSourceScheme)) {
-    inner_url = GURL(gurl.GetContent());
-  }
-
-  bool is_file = inner_url.SchemeIsFile();
-#if BUILDFLAG(IS_CHROMEOS)
-  // The "externalfile" scheme is ChromeOS-specific.
-  is_file |= inner_url.SchemeIs(content::kExternalFileScheme);
-#endif
-
-  if (is_file && !may_read_local_files_) {
-    callback->sendFailure(
-        Response::ServerError("Navigating to local URL is not allowed"));
-    return;
-  }
-
   if (!host_) {
     callback->sendFailure(Response::InternalError());
     return;
   }
 
-  // chrome-untrusted:// WebUIs might perform high-priviledged actions on
-  // navigation, disallow navigation to them unless the client is trusted.
-  if ((inner_url.SchemeIs(kChromeUIUntrustedScheme) ||
-       inner_url.SchemeIs(kChromeDevToolsScheme)) &&
-      !is_trusted_) {
-    callback->sendFailure(Response::ServerError(
-        "Navigating to a URL with a privileged scheme is not allowed"));
+  Response response = CheckNavigationAllowed(gurl);
+  if (response.IsError()) {
+    callback->sendFailure(std::move(response));
     return;
   }
 
-  if (!session()->GetClient()->MayAttachToURL(gurl,
-                                              host_->web_ui() != nullptr)) {
-    url::Origin origin = url::Origin::Create(gurl);
-    if (!origin.scheme().empty() &&
-        origin.scheme() != content::kChromeUIScheme &&
-        origin.scheme() != content::kChromeUIUntrustedScheme &&
-        origin.scheme() != content::kChromeDevToolsScheme) {
-      callback->sendFailure(Response::ServerError("Not allowed"));
-      return;
-    }
-  }
-
   ui::PageTransition type;
   std::string transition_type =
       maybe_transition_type.value_or(Page::TransitionTypeEnum::Typed);
@@ -1269,12 +1277,22 @@
   *entries = std::make_unique<NavigationEntries>();
   for (int i = 0; i != controller.GetEntryCount(); ++i) {
     auto* entry = controller.GetEntryAtIndex(i);
+    // A client that is not allowed to navigate the page to a URL must not
+    // learn it either, so the same check gates disclosure. Redact such entries
+    // rather than omitting them, so that `currentIndex` and the entry ids keep
+    // matching the actual history. Navigating to a redacted entry is rejected
+    // by NavigateToHistoryEntry().
+    const bool may_disclose =
+        !CheckNavigationAllowed(entry->GetURL()).IsError() &&
+        !CheckNavigationAllowed(entry->GetUserTypedURL()).IsError();
     (*entries)->emplace_back(
         Page::NavigationEntry::Create()
             .SetId(entry->GetUniqueID())
-            .SetUrl(entry->GetURL().spec())
-            .SetUserTypedURL(entry->GetUserTypedURL().spec())
-            .SetTitle(base::UTF16ToUTF8(entry->GetTitle()))
+            .SetUrl(may_disclose ? entry->GetURL().spec() : std::string())
+            .SetUserTypedURL(may_disclose ? entry->GetUserTypedURL().spec()
+                                          : std::string())
+            .SetTitle(may_disclose ? base::UTF16ToUTF8(entry->GetTitle())
+                                   : std::string())
             .SetTransitionType(TransitionTypeName(entry->GetTransitionType()))
             .Build());
   }
@@ -1289,10 +1307,18 @@
 
   NavigationController& controller = host_->frame_tree()->controller();
   for (int i = 0; i != controller.GetEntryCount(); ++i) {
-    if (controller.GetEntryAtIndex(i)->GetUniqueID() == entry_id) {
-      controller.GoToIndex(i);
-      return Response::Success();
+    NavigationEntry* entry = controller.GetEntryAtIndex(i);
+    if (entry->GetUniqueID() != entry_id) {
+      continue;
     }
+    // Traversing to an entry navigates the page to the entry's URL, so apply
+    // the same restrictions Navigate() applies.
+    response = CheckNavigationAllowed(entry->GetURL());
+    if (response.IsError()) {
+      return response;
+    }
+    controller.GoToIndex(i);
+    return Response::Success();
   }
 
   return Response::InvalidParams("No entry with passed id");
diff --git a/content/browser/devtools/protocol/page_handler.h b/content/browser/devtools/protocol/page_handler.h
index 15be623..26067a3 100644
--- a/content/browser/devtools/protocol/page_handler.h
+++ b/content/browser/devtools/protocol/page_handler.h
@@ -269,6 +269,11 @@
   using ResponseOrWebContents = std::variant<Response, WebContentsImpl*>;
   ResponseOrWebContents GetWebContentsForTopLevelActiveFrame();
 
+  // Returns an error response if the client's trust level or local file access
+  // permission does not allow it to navigate the page to `url`. `host_` must
+  // not be null.
+  Response CheckNavigationAllowed(const GURL& url);
+
   const bool allow_unsafe_operations_;
   const bool is_trusted_;
   const std::optional<url::Origin> navigation_initiator_origin_;
diff --git a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
index 9cb28a5..0ae6495 100644
--- a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
+++ b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
@@ -2079,6 +2079,87 @@
 #endif
 
 IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest,
+                       GetNavigationHistoryRedactsFileUrlsWithoutFileAccess) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  const GURL file_url = GetTestUrl("devtools", "navigation.html");
+  const GURL http_url = embedded_test_server()->GetURL("/title1.html");
+  NavigateToURLBlockUntilNavigationsComplete(shell(), file_url, 1);
+  NavigateToURLBlockUntilNavigationsComplete(shell(), http_url, 1);
+
+  // With file access, the file:// entry is exposed.
+  Attach();
+  const base::ListValue* entries =
+      SendCommandSync("Page.getNavigationHistory")->FindList("entries");
+  ASSERT_TRUE(entries);
+  ASSERT_EQ(2u, entries->size());
+  EXPECT_EQ(file_url.spec(), *(*entries)[0].GetDict().FindString("url"));
+
+  Detach();
+  SetMayReadLocalFiles(false);
+  Attach();
+
+  // Without file access, the file:// entry is redacted, but it is still listed
+  // so that the remaining entries and `currentIndex` stay meaningful.
+  entries = SendCommandSync("Page.getNavigationHistory")->FindList("entries");
+  ASSERT_TRUE(entries);
+  ASSERT_EQ(2u, entries->size());
+  const base::DictValue& file_entry = (*entries)[0].GetDict();
+  EXPECT_EQ("", *file_entry.FindString("url"));
+  EXPECT_EQ("", *file_entry.FindString("userTypedURL"));
+  EXPECT_EQ("", *file_entry.FindString("title"));
+  EXPECT_EQ(http_url.spec(), *(*entries)[1].GetDict().FindString("url"));
+}
+
+IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest,
+                       NavigateToHistoryEntryWithFileUrlRequiresFileAccess) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  const GURL file_url = GetTestUrl("devtools", "navigation.html");
+  const GURL first_http_url = embedded_test_server()->GetURL("/title1.html");
+  const GURL second_http_url = embedded_test_server()->GetURL("/title2.html");
+  NavigateToURLBlockUntilNavigationsComplete(shell(), file_url, 1);
+  NavigateToURLBlockUntilNavigationsComplete(shell(), first_http_url, 1);
+  NavigateToURLBlockUntilNavigationsComplete(shell(), second_http_url, 1);
+
+  Attach();
+  const base::ListValue* entries =
+      SendCommandSync("Page.getNavigationHistory")->FindList("entries");
+  ASSERT_TRUE(entries);
+  ASSERT_EQ(3u, entries->size());
+  const std::optional<int> file_entry_id =
+      (*entries)[0].GetDict().FindInt("id");
+  ASSERT_TRUE(file_entry_id.has_value());
+  const std::optional<int> http_entry_id =
+      (*entries)[1].GetDict().FindInt("id");
+  ASSERT_TRUE(http_entry_id.has_value());
+
+  Detach();
+  SetMayReadLocalFiles(false);
+  Attach();
+
+  // Traversing to the file:// entry is rejected, just like navigating to it
+  // with Page.navigate would be.
+  base::DictValue params;
+  params.Set("entryId", *file_entry_id);
+  ASSERT_FALSE(
+      SendCommandSync("Page.navigateToHistoryEntry", std::move(params)));
+  EXPECT_THAT(
+      error()->FindInt("code"),
+      testing::Optional(static_cast<int>(crdtp::DispatchCode::SERVER_ERROR)));
+  EXPECT_EQ(*error()->FindString("message"),
+            "Navigating to local URL is not allowed");
+  EXPECT_EQ(second_http_url, shell()->web_contents()->GetLastCommittedURL());
+
+  // Traversing to an entry the client is allowed to access still works.
+  base::DictValue allowed_params;
+  allowed_params.Set("entryId", *http_entry_id);
+  TestNavigationObserver navigation_observer(shell()->web_contents());
+  ASSERT_TRUE(SendCommandSync("Page.navigateToHistoryEntry",
+                              std::move(allowed_params)));
+  navigation_observer.Wait();
+  EXPECT_EQ(first_http_url, shell()->web_contents()->GetLastCommittedURL());
+}
+
+IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest,
                        DOMGetFileInfoRequiresFileAccess) {
   NavigateToURLBlockUntilNavigationsComplete(shell(), GURL("about:blank"), 1);
   Attach();

diff --git a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
index 9cb28a5..0ae6495 100644
--- a/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
+++ b/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
@@ -2079,6 +2079,87 @@
 #endif
 
 IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest,
+                       GetNavigationHistoryRedactsFileUrlsWithoutFileAccess) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  const GURL file_url = GetTestUrl("devtools", "navigation.html");
+  const GURL http_url = embedded_test_server()->GetURL("/title1.html");
+  NavigateToURLBlockUntilNavigationsComplete(shell(), file_url, 1);
+  NavigateToURLBlockUntilNavigationsComplete(shell(), http_url, 1);
+
+  // With file access, the file:// entry is exposed.
+  Attach();
+  const base::ListValue* entries =
+      SendCommandSync("Page.getNavigationHistory")->FindList("entries");
+  ASSERT_TRUE(entries);
+  ASSERT_EQ(2u, entries->size());
+  EXPECT_EQ(file_url.spec(), *(*entries)[0].GetDict().FindString("url"));
+
+  Detach();
+  SetMayReadLocalFiles(false);
+  Attach();
+
+  // Without file access, the file:// entry is redacted, but it is still listed
+  // so that the remaining entries and `currentIndex` stay meaningful.
+  entries = SendCommandSync("Page.getNavigationHistory")->FindList("entries");
+  ASSERT_TRUE(entries);
+  ASSERT_EQ(2u, entries->size());
+  const base::DictValue& file_entry = (*entries)[0].GetDict();
+  EXPECT_EQ("", *file_entry.FindString("url"));
+  EXPECT_EQ("", *file_entry.FindString("userTypedURL"));
+  EXPECT_EQ("", *file_entry.FindString("title"));
+  EXPECT_EQ(http_url.spec(), *(*entries)[1].GetDict().FindString("url"));
+}
+
+IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest,
+                       NavigateToHistoryEntryWithFileUrlRequiresFileAccess) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  const GURL file_url = GetTestUrl("devtools", "navigation.html");
+  const GURL first_http_url = embedded_test_server()->GetURL("/title1.html");
+  const GURL second_http_url = embedded_test_server()->GetURL("/title2.html");
+  NavigateToURLBlockUntilNavigationsComplete(shell(), file_url, 1);
+  NavigateToURLBlockUntilNavigationsComplete(shell(), first_http_url, 1);
+  NavigateToURLBlockUntilNavigationsComplete(shell(), second_http_url, 1);
+
+  Attach();
+  const base::ListValue* entries =
+      SendCommandSync("Page.getNavigationHistory")->FindList("entries");
+  ASSERT_TRUE(entries);
+  ASSERT_EQ(3u, entries->size());
+  const std::optional<int> file_entry_id =
+      (*entries)[0].GetDict().FindInt("id");
+  ASSERT_TRUE(file_entry_id.has_value());
+  const std::optional<int> http_entry_id =
+      (*entries)[1].GetDict().FindInt("id");
+  ASSERT_TRUE(http_entry_id.has_value());
+
+  Detach();
+  SetMayReadLocalFiles(false);
+  Attach();
+
+  // Traversing to the file:// entry is rejected, just like navigating to it
+  // with Page.navigate would be.
+  base::DictValue params;
+  params.Set("entryId", *file_entry_id);
+  ASSERT_FALSE(
+      SendCommandSync("Page.navigateToHistoryEntry", std::move(params)));
+  EXPECT_THAT(
+      error()->FindInt("code"),
+      testing::Optional(static_cast<int>(crdtp::DispatchCode::SERVER_ERROR)));
+  EXPECT_EQ(*error()->FindString("message"),
+            "Navigating to local URL is not allowed");
+  EXPECT_EQ(second_http_url, shell()->web_contents()->GetLastCommittedURL());
+
+  // Traversing to an entry the client is allowed to access still works.
+  base::DictValue allowed_params;
+  allowed_params.Set("entryId", *http_entry_id);
+  TestNavigationObserver navigation_observer(shell()->web_contents());
+  ASSERT_TRUE(SendCommandSync("Page.navigateToHistoryEntry",
+                              std::move(allowed_params)));
+  navigation_observer.Wait();
+  EXPECT_EQ(first_http_url, shell()->web_contents()->GetLastCommittedURL());
+}
+
+IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest,
                        DOMGetFileInfoRequiresFileAccess) {
   NavigateToURLBlockUntilNavigationsComplete(shell(), GURL("about:blank"), 1);
   Attach();