PatchGap
Chrome · Print Preview

CVE-2026-17679

Logic Error in Print Preview
High chrome Logic Error 📄 Reporter bug report 🔧 Commit mapped

Overview

High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
ImpactInsufficient validation of untrusted input in Print Preview
DescriptionInsufficient validation of untrusted input in Print Preview
ComponentPrint Preview
Bug ClassLogic Error
Tracker516430649
Fix commitfacaf16b4098 (chromium/src) +213/-137
CISA KEVNot listed
CreditedGoogle
Disclosed2026-07-29

Changed Functions

FunctionChangeNotes
PrintPreviewDataStore
chrome/browser/printing/print_preview_data_service.h
 modified
IN_PROC_BROWSER_TEST_F
chrome/browser/ui/webui/log_web_ui_url_browsertest.cc
 modified

Files Changed

  • chrome/browser/printing/print_preview_data_service.cc
  • chrome/browser/printing/print_preview_data_service.h
  • chrome/browser/printing/print_view_manager.cc
  • chrome/browser/printing/print_view_manager.h
  • chrome/browser/printing/print_view_manager_unittest.cc
  • chrome/browser/resources/pdf/pdf_viewer_print.ts
  • chrome/browser/ui/webui/log_web_ui_url_browsertest.cc

Patch

view commit
From facaf16b4098dec7b26cc6b71339b665e0ceb40f Mon Sep 17 00:00:00 2001
From: Lei Zhang <thestig@chromium.org>
Date: Mon, 29 Jun 2026 16:47:21 -0700
Subject: [PATCH] Print Preview: Change ui_id from int to base::UnguessableToken

Make chrome-untrusted://print/.../print.pdf URLs harder to guess.

- In `PrintManagerHost`, change the PrintPreviewUI ID type from `int` to
  `base::UnguessableToken` to improve security and uniqueness. Then
  update subclasses and tests to match the print.mojom changes.
- Update `PrintPreviewIdAndPageIndex()` and `ParseDataPath()` to use
  `base::UnguessableToken`.
- Refactor `PrintPreviewUI` to use non-optional `base::UnguessableToken`
  for `id_`, removing redundant invalid states, and update related
  methods and callers.
- Change `PrintRenderFrameHelper::preview_ui_id_` to a
  `base::UnguessableToken` accordingly.
- Update various test values and comments to refer to the new format.

Of course, the human did some of the work, because it was faster than
explaining exactly what should be done to the AI. Hooray for getting
participation points.

TAG=agy
CONV=95d5fab0-51c2-4421-a161-46356b36d772

Bug: 516430649
Change-Id: I7fdc3a40ebea78e5ad35aaea711b1da1925c53c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7900625
Reviewed-by: Alex Gough <ajgo@chromium.org>
Reviewed-by: April Kallmeyer <ask@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1654467}
---

diff --git a/chrome/browser/printing/print_preview_data_service.cc b/chrome/browser/printing/print_preview_data_service.cc
index 4056c0b..5b0a0dd0 100644
--- a/chrome/browser/printing/print_preview_data_service.cc
+++ b/chrome/browser/printing/print_preview_data_service.cc
@@ -96,7 +96,7 @@
 PrintPreviewDataService::~PrintPreviewDataService() = default;
 
 scoped_refptr<base::RefCountedMemory> PrintPreviewDataService::GetDataEntry(
-    int32_t preview_ui_id,
+    const base::UnguessableToken& preview_ui_id,
     int index) const {
   auto it = data_store_map_.find(preview_ui_id);
   return it != data_store_map_.end() ? it->second->GetPreviewDataForIndex(index)
@@ -104,7 +104,7 @@
 }
 
 void PrintPreviewDataService::SetDataEntry(
-    int32_t preview_ui_id,
+    const base::UnguessableToken& preview_ui_id,
     int index,
     scoped_refptr<base::RefCountedMemory> data_bytes) {
   if (!data_store_map_.contains(preview_ui_id)) {
@@ -114,6 +114,7 @@
                                                          std::move(data_bytes));
 }
 
-void PrintPreviewDataService::RemoveEntry(int32_t preview_ui_id) {
+void PrintPreviewDataService::RemoveEntry(
+    const base::UnguessableToken& preview_ui_id) {
   data_store_map_.erase(preview_ui_id);
 }
diff --git a/chrome/browser/printing/print_preview_data_service.h b/chrome/browser/printing/print_preview_data_service.h
index 317bab8c..6643657 100644
--- a/chrome/browser/printing/print_preview_data_service.h
+++ b/chrome/browser/printing/print_preview_data_service.h
@@ -5,12 +5,11 @@
 #ifndef CHROME_BROWSER_PRINTING_PRINT_PREVIEW_DATA_SERVICE_H_
 #define CHROME_BROWSER_PRINTING_PRINT_PREVIEW_DATA_SERVICE_H_
 
-#include <stdint.h>
-
 #include <map>
 #include <memory>
 
 #include "base/memory/ref_counted.h"
+#include "base/unguessable_token.h"
 
 class PrintPreviewDataStore;
 
@@ -33,18 +32,19 @@
   // `printing::COMPLETE_PREVIEW_DOCUMENT_INDEX` to represent complete preview
   // data. Use `index` to retrieve a specific preview page data.
   // Returns nullptr if the requested page is not yet available.
-  scoped_refptr<base::RefCountedMemory> GetDataEntry(int32_t preview_ui_id,
-                                                     int index) const;
+  scoped_refptr<base::RefCountedMemory> GetDataEntry(
+      const base::UnguessableToken& preview_ui_id,
+      int index) const;
 
   // Sets/Updates the data entry in PrintPreviewDataStore. `index` is zero-based
   // or `printing::COMPLETE_PREVIEW_DOCUMENT_INDEX` to represent complete
   // preview data. Use `index` to set/update a specific preview page data.
-  void SetDataEntry(int32_t preview_ui_id,
+  void SetDataEntry(const base::UnguessableToken& preview_ui_id,
                     int index,
                     scoped_refptr<base::RefCountedMemory> data);
 
   // Removes the corresponding PrintPreviewUI entry from the map.
-  void RemoveEntry(int32_t preview_ui_id);
+  void RemoveEntry(const base::UnguessableToken& preview_ui_id);
 
  private:
   friend struct base::DefaultSingletonTraits<PrintPreviewDataService>;
@@ -53,7 +53,7 @@
   // Key: PrintPreviewUI ID.
   // Value: Print preview data store object.
   using PreviewDataStoreMap =
-      std::map<int32_t, std::unique_ptr<PrintPreviewDataStore>>;
+      std::map<base::UnguessableToken, std::unique_ptr<PrintPreviewDataStore>>;
 
   PrintPreviewDataService();
   ~PrintPreviewDataService();
diff --git a/chrome/browser/printing/print_view_manager.cc b/chrome/browser/printing/print_view_manager.cc
index 39362225..f91135d 100644
--- a/chrome/browser/printing/print_view_manager.cc
+++ b/chrome/browser/printing/print_view_manager.cc
@@ -647,9 +647,10 @@
   PrintPreviewAllowedForTesting();
 }
 
-void PrintViewManager::CheckForCancel(int32_t preview_ui_id,
-                                      int32_t request_id,
-                                      CheckForCancelCallback callback) {
+void PrintViewManager::CheckForCancel(
+    const base::UnguessableToken& preview_ui_id,
+    int32_t request_id,
+    CheckForCancelCallback callback) {
   std::move(callback).Run(
       PrintPreviewUI::ShouldCancelRequest(preview_ui_id, request_id));
 }
diff --git a/chrome/browser/printing/print_view_manager.h b/chrome/browser/printing/print_view_manager.h
index 66590f93..599963a 100644
--- a/chrome/browser/printing/print_view_manager.h
+++ b/chrome/browser/printing/print_view_manager.h
@@ -81,7 +81,7 @@
       SetupScriptedPrintPreviewCallback callback) override;
   void ShowScriptedPrintPreview() override;
   void RequestPrintPreview(mojom::RequestPrintPreviewParamsPtr params) override;
-  void CheckForCancel(int32_t preview_ui_id,
+  void CheckForCancel(const base::UnguessableToken& preview_ui_id,
                       int32_t request_id,
                       CheckForCancelCallback callback) override;
   void SetAccessibilityTree(
diff --git a/chrome/browser/printing/print_view_manager_unittest.cc b/chrome/browser/printing/print_view_manager_unittest.cc
index 47270b3c..0ca9163 100644
--- a/chrome/browser/printing/print_view_manager_unittest.cc
+++ b/chrome/browser/printing/print_view_manager_unittest.cc
@@ -354,7 +354,7 @@
       mojom::RequestPrintPreviewParamsPtr params) override {
     NOTREACHED();
   }
-  void CheckForCancel(int32_t preview_ui_id,
+  void CheckForCancel(const base::UnguessableToken& preview_ui_id,
                       int32_t request_id,
                       CheckForCancelCallback callback) override {
     NOTREACHED();
diff --git a/chrome/browser/resources/pdf/pdf_viewer_print.ts b/chrome/browser/resources/pdf/pdf_viewer_print.ts
index bdff2ad..85e3640 100644
--- a/chrome/browser/resources/pdf/pdf_viewer_print.ts
+++ b/chrome/browser/resources/pdf/pdf_viewer_print.ts
@@ -79,7 +79,7 @@
               assert(paths.length === 4);
               assert(paths[3] === 'print.pdf');
               // Valid Print Preview UI ID
-              assert(!Number.isNaN(parseInt(paths[1]!)));
+              assert(/^[0-9a-fA-F]{32}$/.test(paths[1]!));
               // Valid page index (can be negative for PDFs).
               assert(!Number.isNaN(parseInt(paths[2]!)));
               return url.toString();
diff --git a/chrome/browser/ui/webui/log_web_ui_url_browsertest.cc b/chrome/browser/ui/webui/log_web_ui_url_browsertest.cc
index 902bfdf5..8cc737a1 100644
--- a/chrome/browser/ui/webui/log_web_ui_url_browsertest.cc
+++ b/chrome/browser/ui/webui/log_web_ui_url_browsertest.cc
@@ -12,6 +12,7 @@
 #include "base/strings/strcat.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "base/unguessable_token.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
@@ -94,8 +95,9 @@
 
 #if !BUILDFLAG(IS_ANDROID)
 IN_PROC_BROWSER_TEST_F(LogWebUIUrlTest, TestChromeUntrustedPage) {
-  RunTest(u"", GURL(base::StrCat(
-                   {chrome::kChromeUIUntrustedPrintURL, "1/1/print.pdf"})));
+  RunTest(u"", GURL(base::StrCat({chrome::kChromeUIUntrustedPrintURL,
+                                  base::UnguessableToken::Create().ToString(),
+                                  "/1/print.pdf"})));
 }
 #endif
 
diff --git a/chrome/browser/ui/webui/print_preview/parse_data_path.cc b/chrome/browser/ui/webui/print_preview/parse_data_path.cc
index 92c2a62..2efcdaa 100644
Loading diff…

Regression Test / PoC

shipped with the fix
diff --git a/chrome/browser/printing/print_view_manager_unittest.cc b/chrome/browser/printing/print_view_manager_unittest.cc
index 47270b3c..0ca9163 100644
--- a/chrome/browser/printing/print_view_manager_unittest.cc
+++ b/chrome/browser/printing/print_view_manager_unittest.cc
@@ -354,7 +354,7 @@
       mojom::RequestPrintPreviewParamsPtr params) override {
     NOTREACHED();
   }
-  void CheckForCancel(int32_t preview_ui_id,
+  void CheckForCancel(const base::UnguessableToken& preview_ui_id,
                       int32_t request_id,
                       CheckForCancelCallback callback) override {
     NOTREACHED();
diff --git a/chrome/browser/ui/webui/log_web_ui_url_browsertest.cc b/chrome/browser/ui/webui/log_web_ui_url_browsertest.cc
index 902bfdf5..8cc737a1 100644
--- a/chrome/browser/ui/webui/log_web_ui_url_browsertest.cc
+++ b/chrome/browser/ui/webui/log_web_ui_url_browsertest.cc
@@ -12,6 +12,7 @@
 #include "base/strings/strcat.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "base/unguessable_token.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
@@ -94,8 +95,9 @@
 
 #if !BUILDFLAG(IS_ANDROID)
 IN_PROC_BROWSER_TEST_F(LogWebUIUrlTest, TestChromeUntrustedPage) {
-  RunTest(u"", GURL(base::StrCat(
-                   {chrome::kChromeUIUntrustedPrintURL, "1/1/print.pdf"})));
+  RunTest(u"", GURL(base::StrCat({chrome::kChromeUIUntrustedPrintURL,
+                                  base::UnguessableToken::Create().ToString(),
+                                  "/1/print.pdf"})));
 }
 #endif
diff --git a/chrome/browser/ui/webui/print_preview/parse_data_path_unittest.cc b/chrome/browser/ui/webui/print_preview/parse_data_path_unittest.cc
index 8b59878..f331355 100644
--- a/chrome/browser/ui/webui/print_preview/parse_data_path_unittest.cc
+++ b/chrome/browser/ui/webui/print_preview/parse_data_path_unittest.cc
@@ -6,43 +6,49 @@
 
 #include <optional>
 
+#include "base/unguessable_token.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace printing {
 
 TEST(ParseDataPathTest, ParseDataPath) {
+  base::UnguessableToken token = base::UnguessableToken::Create();
   std::optional<PrintPreviewIdAndPageIndex> parsed =
-      ParseDataPath("3/4/print.pdf");
+      ParseDataPath(token.ToString() + "/4/print.pdf");
   ASSERT_TRUE(parsed);
 
-  EXPECT_EQ(parsed->ui_id, 3);
+  EXPECT_EQ(parsed->ui_id, token);
   EXPECT_EQ(parsed->page_index, 4);
 }
 
 TEST(ParseDataPathTest, ParseDataPathTest) {
   std::optional<PrintPreviewIdAndPageIndex> parsed =
-      ParseDataPath("1/1/test.pdf");
+      ParseDataPath("123456789abcdef00fedcba987654321/0/test.pdf");
   ASSERT_TRUE(parsed);
 
-  EXPECT_EQ(parsed->ui_id, -1);
+  EXPECT_EQ(parsed->ui_id, base::UnguessableToken::Deserialize(
+                               0x123456789abcdef0, 0x0fedcba987654321)
+                               .value());
   EXPECT_EQ(parsed->page_index, 0);
 }
 
 TEST(ParseDataPathTest, ParseDataPathValid) {
-  EXPECT_TRUE(ParseDataPath("1/2/print.pdf"));
+  base::UnguessableToken token = base::UnguessableToken::Create();
+  EXPECT_TRUE(ParseDataPath(token.ToString() + "/2/print.pdf"));
 }
 
 TEST(ParseDataPathTest, ParseDataPathInvalid) {
   // Doesn't end in print.pdf
   EXPECT_FALSE(ParseDataPath("pdf/browser_api.js"));
   // Doesn't have both page index and UI ID.
-  EXPECT_FALSE(ParseDataPath("1/print.pdf"));
-  // Non-integer UI ID
-  EXPECT_FALSE(ParseDataPath("foo/0/print.pdf"));
-  // UI ID < 0
-  EXPECT_FALSE(ParseDataPath("-1/0/print.pdf"));
+  EXPECT_FALSE(ParseDataPath("1234567890abcdef1234567890abcdef/print.pdf"));
+  // Invalid UI ID (not hex)
+  EXPECT_FALSE(ParseDataPath("z234567890abcdef1234567890abcdef/0/print.pdf"));
+  // Invalid UI ID (too short)
+  EXPECT_FALSE(ParseDataPath("1234567890abcdef1234567890abcde/0/print.pdf"));
   // Non-integer page index
-  EXPECT_FALSE(ParseDataPath("1/foo/print.pdf"));
+  base::UnguessableToken token = base::UnguessableToken::Create();
+  EXPECT_FALSE(ParseDataPath(token.ToString() + "/foo/print.pdf"));
 }
 
 }  // namespace printing
diff --git a/chrome/browser/ui/webui/print_preview/print_preview_handler_unittest.cc b/chrome/browser/ui/webui/print_preview/print_preview_handler_unittest.cc
index 4117145..ba36711 100644
--- a/chrome/browser/ui/webui/print_preview/print_preview_handler_unittest.cc
+++ b/chrome/browser/ui/webui/print_preview/print_preview_handler_unittest.cc
@@ -29,6 +29,7 @@
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/values_test_util.h"
+#include "base/unguessable_token.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "chrome/browser/printing/print_preview_dialog_controller.h"
@@ -1290,9 +1291,11 @@
   ASSERT_TRUE(request_value.has_value());
   int preview_request_id = request_value.value();
 
-  std::optional<int> ui_value = preview_params.FindInt(kPreviewUIID);
-  ASSERT_TRUE(ui_value.has_value());
-  int preview_ui_id = ui_value.value();
+  const std::string* ui_value = preview_params.FindString(kPreviewUIID);
+  ASSERT_TRUE(ui_value);
+  std::optional<base::UnguessableToken> preview_ui_id =
+      base::UnguessableToken::DeserializeFromString(*ui_value);
+  ASSERT_TRUE(preview_ui_id.has_value());
 
   // Simulate renderer responses: PageLayoutReady, PageCountReady,
   // PagePreviewReady, and OnPrintPreviewReady will be called in that order.
@@ -1320,11 +1323,11 @@
   AssertWebUIEventFired(*web_ui()->call_data().back(), "page-count-ready");
 
   // Page at index 0 is ready.
-  handler()->SendPagePreviewReady(0, preview_ui_id, preview_request_id);
+  handler()->SendPagePreviewReady(0, preview_ui_id.value(), preview_request_id);
   AssertWebUIEventFired(*web_ui()->call_data().back(), "page-preview-ready");
 
   // Print preview is ready.
-  handler()->OnPrintPreviewReady(preview_ui_id, preview_request_id);
+  handler()->OnPrintPreviewReady(preview_ui_id.value(), preview_request_id);
   CheckWebUIResponse(*web_ui()->call_data().back(), callback_id_in, true);
 
   // Renderer responses have been as expected.
@@ -1340,7 +1343,7 @@
   EXPECT_EQ(message_count, web_ui()->call_data().size());
   handler()->SendPageCountReady(1, -1, 0);
   EXPECT_EQ(message_count, web_ui()->call_data().size());
-  handler()->OnPrintPreviewReady(0, 0);
+  handler()->OnPrintPreviewReady(base::UnguessableToken(), 0);
   EXPECT_EQ(message_count, web_ui()->call_data().size());
 
   // Handler should have tried to kill the renderer for each of these.
diff --git a/chrome/browser/ui/webui/print_preview/print_preview_ui_browsertest.cc b/chrome/browser/ui/webui/print_preview/print_preview_ui_browsertest.cc
index f242ccf6..07afd4b3 100644
--- a/chrome/browser/ui/webui/print_preview/print_preview_ui_browsertest.cc
+++ b/chrome/browser/ui/webui/print_preview/print_preview_ui_browsertest.cc
@@ -2,10 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "chrome/browser/ui/webui/print_preview/print_preview_ui.h"
+
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "build/build_config.h"
 #include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/printing/print_preview_dialog_controller.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/task_manager/task_manager_browsertest_util.h"
 #include "chrome/browser/ui/browser.h"
@@ -20,6 +23,7 @@
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
+#include "content/public/browser/web_ui.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
@@ -163,7 +167,23 @@
   content::TestNavigationObserver nav_observer(nullptr);
   nav_observer.WatchExistingWebContents();
   nav_observer.Wait();
-  EXPECT_EQ(GURL("chrome-untrusted://print/1/0/print.pdf"),
+
+  // Get the print preview UI ID to construct the expected URL.
+  content::WebContents* initiator =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(initiator);
+  content::WebContents* preview_dialog =
+      printing::PrintPreviewDialogController::GetInstance()
+          ->GetPrintPreviewForContents(initiator);
+  ASSERT_TRUE(preview_dialog);
+  content::WebUI* web_ui = preview_dialog->GetWebUI();
+  ASSERT_TRUE(web_ui);
+  auto* print_preview_ui =
+      web_ui->GetController()->GetAs<printing::PrintPreviewUI>();
+  ASSERT_TRUE(print_preview_ui);
+  std::string ui_id = print_preview_ui->GetIDForPrintPreviewUI().ToString();
+
+  EXPECT_EQ(GURL("chrome-untrusted://print/" + ui_id + "/0/print.pdf"),
             nav_observer.last_navigation_url());
   histogram_tester.ExpectBucketCount(
       "PrintPreview.UserAction", printing::UserActionBuckets::kPreviewStarted,
diff --git a/chrome/browser/ui/webui/print_preview/print_preview_ui_unittest.cc b/chrome/browser/ui/webui/print_preview/print_preview_ui_unittest.cc
index 3ebf0e9..cfdf62c 100644
--- a/chrome/browser/ui/webui/print_preview/print_preview_ui_unittest.cc
+++ b/chrome/browser/ui/webui/print_preview/print_preview_ui_unittest.cc
@@ -226,7 +226,7 @@
 
   // Test the initial state.
   EXPECT_TRUE(PrintPreviewUI::ShouldCancelRequest(
-      *preview_ui->GetIDForPrintPreviewUI(), 0));
+      preview_ui->GetIDForPrintPreviewUI(), 0));
 
   const int kFirstRequestId = 1000;
   const int kSecondRequestId = 1001;
@@ -234,16 +234,16 @@
   // Test with kFirstRequestId.
   preview_ui->OnPrintPreviewRequest(kFirstRequestId);
   EXPECT_FALSE(PrintPreviewUI::ShouldCancelRequest(
-      *preview_ui->GetIDForPrintPreviewUI(), kFirstRequestId));
+      preview_ui->GetIDForPrintPreviewUI(), kFirstRequestId));
   EXPECT_TRUE(PrintPreviewUI::ShouldCancelRequest(
-      *preview_ui->GetIDForPrintPreviewUI(), kSecondRequestId));
+      preview_ui->GetIDForPrintPreviewUI(), kSecondRequestId));
 
   // Test with kSecondRequestId.
   preview_ui->OnPrintPreviewRequest(kSecondRequestId);
   EXPECT_TRUE(PrintPreviewUI::ShouldCancelRequest(
-      *preview_ui->GetIDForPrintPreviewUI(), kFirstRequestId));
+      preview_ui->GetIDForPrintPreviewUI(), kFirstRequestId));
   EXPECT_FALSE(PrintPreviewUI::ShouldCancelRequest(
-      *preview_ui->GetIDForPrintPreviewUI(), kSecondRequestId));
+      preview_ui->GetIDForPrintPreviewUI(), kSecondRequestId));
 }
 
 // Ensures that a failure cancels all pending actions.
diff --git a/chrome/browser/ui/webui/print_preview/print_preview_ui_untrusted_unittest.cc b/chrome/browser/ui/webui/print_preview/print_preview_ui_untrusted_unittest.cc
index 89e84527..cab4211 100644
--- a/chrome/browser/ui/webui/print_preview/print_preview_ui_untrusted_unittest.cc
+++ b/chrome/browser/ui/webui/print_preview/print_preview_ui_untrusted_unittest.cc
@@ -14,11 +14,13 @@
 #include "base/memory/scoped_refptr.h"
 #include "base/path_service.h"
 #include "base/threading/thread_restrictions.h"
+#include "base/unguessable_token.h"
 #include "chrome/browser/printing/print_preview_data_service.h"
 #include "chrome/browser/printing/print_preview_test.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/webui/print_preview/parse_data_path.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "content/public/browser/web_contents.h"
@@ -54,18 +56,22 @@
 TEST_F(PrintPreviewUIUntrustedUnitTest, PrintPreviewData) {
   PrintPreviewDataService* data_service =
       PrintPreviewDataService::GetInstance();
-  scoped_refptr<base::RefCountedMemory> dummy_data = CreateTestData();
-  data_service->SetDataEntry(0, 0, dummy_data.get());
+  scoped_refptr<base::RefCountedMemory> test_data = CreateTestData();
+  base::UnguessableToken token1 = base::UnguessableToken::Create();
+  data_service->SetDataEntry(token1, 0, test_data.get());
 
   // Valid request for data in the data service.
   scoped_refptr<base::RefCountedMemory> data =
-      PrintPreviewUIUntrusted::GetPrintPreviewDataForTest("0/0/print.pdf");
+      PrintPreviewUIUntrusted::GetPrintPreviewDataForTest(token1.ToString() +
+                                                          "/0/print.pdf");
   ASSERT_TRUE(data);
-  EXPECT_EQ(dummy_data->size(), data->size());
-  EXPECT_EQ(dummy_data.get(), data.get());
+  EXPECT_EQ(test_data->size(), data->size());
+  EXPECT_EQ(test_data.get(), data.get());
 
   // Invalid request
-  data = PrintPreviewUIUntrusted::GetPrintPreviewDataForTest("1/0/print.pdf");
+  base::UnguessableToken token2 = base::UnguessableToken::Create();
+  data = PrintPreviewUIUntrusted::GetPrintPreviewDataForTest(token2.ToString() +
+                                                             "/0/print.pdf");
   ASSERT_TRUE(data);
   EXPECT_EQ(0u, data->size());
 
@@ -77,11 +83,12 @@
   base::FilePath pdf_path =
       test_data_path.AppendASCII("pdf/test.pdf").NormalizePathSeparators();
   ASSERT_TRUE(base::ReadFileToString(pdf_path, &test_pdf_content));
-  auto test_data =
+  auto test_pdf_data =
       base::MakeRefCounted<base::RefCountedString>(std::move(test_pdf_content));
-  data = PrintPreviewUIUntrusted::GetPrintPreviewDataForTest("0/0/test.pdf");
+  data = PrintPreviewUIUntrusted::GetPrintPreviewDataForTest(
+      "123456789abcdef00fedcba987654321/0/test.pdf");
   ASSERT_TRUE(data);
-  EXPECT_EQ(test_data->size(), data->size());
-  EXPECT_TRUE(data->Equals(test_data));
+  EXPECT_EQ(test_pdf_data->size(), data->size());
+  EXPECT_TRUE(data->Equals(test_pdf_data));
 }
 }  // namespace printing
... (truncated)
Loading diff…

Original Bug Report

reported by vm...@google.com

IDOR in PrintPreviewUIUntrusted allows cross-profile/cross-origin PDF disclosure

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 Insecure Direct Object Reference (IDOR) exists in chrome-untrusted://print/ due to sequential ID allocation and lack of frame-ownership validation. A compromised chrome-untrusted renderer could potentially fetch and disclose the rendered PDF contents of any concurrently open Print Preview dialog across different origins, profiles, or Incognito sessions. This occurs because the PDF storage backend is managed by a process-global singleton that does not validate the requesting context.

Affected files:

  • chrome/browser/ui/webui/print_preview/print_preview_ui_untrusted.cc
  • chrome/browser/printing/print_preview_data_service.cc
  • chrome/browser/ui/webui/print_preview/parse_data_path.cc
  • chrome/browser/ui/webui/print_preview/print_preview_ui.cc

Estimated timestamp from git blame: 2021-10-08

Root Cause

In chrome/browser/ui/webui/print_preview/print_preview_ui_untrusted.cc, the request filter for chrome-untrusted://print/ is registered with a static callback handler that receives only the URL path string. It completely lacks the context of the requesting frame or WebContents and does not check whether the requesting frame is associated with or authorized to access the requested ID:

// chrome/browser/ui/webui/print_preview/print_preview_ui_untrusted.cc:42
PrintPreviewUIUntrusted::PrintPreviewUIUntrusted(content::WebUI* web_ui)
    : UntrustedWebUIController(web_ui) {
  content::WebUIDataSource* source = content::WebUIDataSource::CreateAndAdd(
      web_ui->GetWebContents()->GetBrowserContext(),
      chrome::kChromeUIUntrustedPrintURL);
  source->SetRequestFilter(base::BindRepeating(&ShouldHandleRequestCallback),
                           base::BindRepeating(&HandleRequestCallback));
}

The static callback extracts the parameters via ParseDataPath and retrieves the PDF data using a process-global singleton PrintPreviewDataService:

// chrome/browser/ui/webui/print_preview/print_preview_ui_untrusted.cc:55
scoped_refptr<base::RefCountedMemory>
PrintPreviewUIUntrusted::GetPrintPreviewData(const std::string& path) {
  std::optional<PrintPreviewIdAndPageIndex> parsed = ParseDataPath(path);
  CHECK(parsed);

  scoped_refptr<base::RefCountedMemory> data =
      PrintPreviewDataService::GetInstance()->GetDataEntry(parsed->ui_id,
                                                           parsed->page_index);
  if (data) {
    return data;
  }
  ...
}

The validation performed by ParseDataPath in chrome/browser/ui/webui/print_preview/parse_data_path.cc (line 17) only ensures that ui_id >= 0 and that the indices are valid integer values.

Crucially, the backend store PrintPreviewDataService is a process-global base::Singleton that has no BrowserContext or profile-level keying:

// chrome/browser/printing/print_preview_data_service.cc:109
PrintPreviewDataService* PrintPreviewDataService::GetInstance() {
  return base::Singleton<PrintPreviewDataService>::get();
}

Additionally, the preview_ui_id keys are sequential small integers allocated from a process-global base::IDMap starting at 1 and incrementing monotonically:

// chrome/browser/ui/webui/print_preview/print_preview_ui.cc:1189
id_ = g_print_preview_ui_id_map.Get().Add(this);

Potential Attack Scenario

Under Chrome’s threat model for chrome-untrusted:// pages, a compromised chrome-untrusted://print/ renderer is treated as a realistic precondition since it hosts PDFium in-process and parses untrusted, attacker-controlled PDF data.

We suggest the following potential steps an attacker could follow to trigger and exploit this vulnerability:

  1. An attacker initiates Print Preview on their own malicious page, causing a chrome-untrusted://print/ subresource/frame to commit.
  2. The attacker exploits a potential vulnerability in the chrome-untrusted renderer (e.g., in PDFium or Blink) to gain arbitrary code execution inside the renderer sandbox.
  3. The compromised renderer bypasses JS WebUI bindings and directly utilizes its bound WebUIURLLoaderFactory for chrome-untrusted:// to issue resource requests for chrome-untrusted://print/<N>/-1/print.pdf (where -1 corresponds to the complete preview document index).
  4. By scanning small sequential integers N, the request reaches the browser-side HandleRequestCallback.
  5. Because there is no frame-ownership, origin, or BrowserContext verification, the browser-side code fetches and returns the complete rendered PDF data belonging to ui_id N from the process-global singleton directly to the attacker’s compromised renderer.

Note: These are potential steps; our current tooling does not have the ability to run code or verify a live proof of concept.

Suggested Fix

  1. Cryptographically Secure Tokens: Instead of exposing and using sequential integers (ui_id) directly in the chrome-untrusted:// URLs, generate a high-entropy cryptographically secure random token (e.g., a 128-bit hex string or UUID) for each print preview session, and require this token in the path to access the PDF data.
  2. Verify Requesting Frame Context: Implement validation on the browser side to ensure that the requesting frame’s origin and BrowserContext match the BrowserContext associated with the print job data. Ensure that only the frame actually hosting the specific print preview is allowed to retrieve its PDF bytes.

Evaluated with Chrome root at commit: a2bea94528f4bd6cc57739c43fa3bb890b8367d3

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.

View on issue tracker
On This Page