From a21bacadecca0d465fbb2f28fabf1590439c06c0 Mon Sep 17 00:00:00 2001
From: Quentin Pubert <[email protected]>
Date: Fri, 31 Jul 2026 05:45:07 -0700
Subject: [PATCH] [iOS][ReaderMode] Block non-trusted iframe requests in Reader Mode

Since Reader Mode is a simplified web page representation, it needs to
prevent tracking or other third-party iframes from loading and leaking
referrers.

This CL introduces a strict iframe domain allowlist matching the trusted
domains allowed by the Readability.js media/video engine.
- Declares a shared regex in C++ constants (kAllowedIframeRegex) in
  ios/chrome/browser/dom_distiller/model/constants.h.
- Introduces DistillerOptions and ReadabilityOptions to cleanly pass
  distillation configuration parameters down to Readability.js and
  DOM Distiller.
- Injects this regex dynamically as a placeholder value
  ($$ALLOWED_VIDEO_REGEX) inside readability_distiller.js.
- Uses this identical regex in the iOS ReaderModeContentTabHelper policy
  decider to block all subframe requests not matching the trusted
  pattern (e.g. YouTube).

Bug: 523243507
Change-Id: I86cc4dbe5ae91a7d7bcf785cf8e1ca5e6a9cb717
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8019571
Reviewed-by: Brandon Wylie <[email protected]>
Reviewed-by: Olivier Robin <[email protected]>
Auto-Submit: Quentin Pubert <[email protected]>
Commit-Queue: Quentin Pubert <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1671776}
---

diff --git a/components/dom_distiller/core/BUILD.gn b/components/dom_distiller/core/BUILD.gn
index 4f3d5124..cb070bf 100644
--- a/components/dom_distiller/core/BUILD.gn
+++ b/components/dom_distiller/core/BUILD.gn
@@ -56,6 +56,8 @@
     "distilled_page_prefs.h",
     "distiller.cc",
     "distiller.h",
+    "distiller_options.cc",
+    "distiller_options.h",
     "distiller_page.cc",
     "distiller_page.h",
     "distiller_ui_handle.h",
@@ -73,6 +75,7 @@
     "extraction_utils.h",
     "page_features.cc",
     "page_features.h",
+    "readability_options.h",
     "task_tracker.cc",
     "task_tracker.h",
     "url_constants.h",
diff --git a/components/dom_distiller/core/distiller.cc b/components/dom_distiller/core/distiller.cc
index 4d7f73f8..a80fe3b 100644
--- a/components/dom_distiller/core/distiller.cc
+++ b/components/dom_distiller/core/distiller.cc
@@ -34,16 +34,22 @@
 
 DistillerFactoryImpl::DistillerFactoryImpl(
     std::unique_ptr<DistillerURLFetcherFactory> distiller_url_fetcher_factory,
+    const DistillerOptions& options)
+    : distiller_url_fetcher_factory_(std::move(distiller_url_fetcher_factory)),
+      options_(options) {}
+
+DistillerFactoryImpl::DistillerFactoryImpl(
+    std::unique_ptr<DistillerURLFetcherFactory> distiller_url_fetcher_factory,
     const dom_distiller::proto::DomDistillerOptions& dom_distiller_options)
     : distiller_url_fetcher_factory_(std::move(distiller_url_fetcher_factory)),
-      dom_distiller_options_(dom_distiller_options) {}
+      options_(dom_distiller_options) {}
 
 DistillerFactoryImpl::~DistillerFactoryImpl() = default;
 
 std::unique_ptr<Distiller> DistillerFactoryImpl::CreateDistiller() {
   // This default implementation has the same behavior for all URLs.
-  std::unique_ptr<DistillerImpl> distiller(new DistillerImpl(
-      *distiller_url_fetcher_factory_, dom_distiller_options_));
+  std::unique_ptr<DistillerImpl> distiller(
+      new DistillerImpl(*distiller_url_fetcher_factory_, options_));
   return std::move(distiller);
 }
 
@@ -53,12 +59,18 @@
 
 DistillerImpl::DistillerImpl(
     const DistillerURLFetcherFactory& distiller_url_fetcher_factory,
-    const dom_distiller::proto::DomDistillerOptions& dom_distiller_options)
+    const DistillerOptions& options)
     : distiller_url_fetcher_factory_(distiller_url_fetcher_factory),
-      dom_distiller_options_(dom_distiller_options),
+      options_(options),
       max_pages_in_article_(kMaxPagesInArticle),
       destruction_allowed_(true) {}
 
+DistillerImpl::DistillerImpl(
+    const DistillerURLFetcherFactory& distiller_url_fetcher_factory,
+    const dom_distiller::proto::DomDistillerOptions& dom_distiller_options)
+    : DistillerImpl(distiller_url_fetcher_factory,
+                    DistillerOptions(dom_distiller_options)) {}
+
 DistillerImpl::~DistillerImpl() {
   DCHECK(destruction_allowed_);
 }
@@ -128,7 +140,7 @@
     // TODO(gilmanmh): Investigate whether this needs to be
     // base::BindRepeating() or if base::BindOnce() can be used instead.
     distiller_page_->DistillPage(
-        url, dom_distiller_options_,
+        url, options_,
         base::BindRepeating(&DistillerImpl::OnPageDistillationFinished,
                             weak_factory_.GetWeakPtr(), page_num, url));
   }
diff --git a/components/dom_distiller/core/distiller.h b/components/dom_distiller/core/distiller.h
index b5051c9..330b398 100644
--- a/components/dom_distiller/core/distiller.h
+++ b/components/dom_distiller/core/distiller.h
@@ -18,9 +18,11 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "components/dom_distiller/core/article_distillation_update.h"
+#include "components/dom_distiller/core/distiller_options.h"
 #include "components/dom_distiller/core/distiller_page.h"
 #include "components/dom_distiller/core/distiller_url_fetcher.h"
 #include "components/dom_distiller/core/proto/distilled_article.pb.h"
+#include "components/dom_distiller/core/readability_options.h"
 #include "third_party/abseil-cpp/absl/container/flat_hash_map.h"
 #include "third_party/abseil-cpp/absl/container/flat_hash_set.h"
 #include "url/gurl.h"
@@ -62,18 +64,23 @@
  public:
   DistillerFactoryImpl(
       std::unique_ptr<DistillerURLFetcherFactory> distiller_url_fetcher_factory,
+      const DistillerOptions& options);
+  DistillerFactoryImpl(
+      std::unique_ptr<DistillerURLFetcherFactory> distiller_url_fetcher_factory,
       const dom_distiller::proto::DomDistillerOptions& dom_distiller_options);
   ~DistillerFactoryImpl() override;
   std::unique_ptr<Distiller> CreateDistiller() override;
 
  private:
   std::unique_ptr<DistillerURLFetcherFactory> distiller_url_fetcher_factory_;
-  dom_distiller::proto::DomDistillerOptions dom_distiller_options_;
+  DistillerOptions options_;
 };
 
 // Distills a article from a page and associated pages.
 class DistillerImpl : public Distiller {
  public:
+  DistillerImpl(const DistillerURLFetcherFactory& distiller_url_fetcher_factory,
+                const DistillerOptions& options);
   DistillerImpl(
       const DistillerURLFetcherFactory& distiller_url_fetcher_factory,
       const dom_distiller::proto::DomDistillerOptions& dom_distiller_options);
@@ -161,7 +168,7 @@
       distiller_url_fetcher_factory_;
   std::unique_ptr<DistillerPage> distiller_page_;
 
-  dom_distiller::proto::DomDistillerOptions dom_distiller_options_;
+  DistillerOptions options_;
   DistillationFinishedCallback finished_cb_;
   DistillationUpdateCallback update_cb_;
 
diff --git a/components/dom_distiller/core/distiller_options.cc b/components/dom_distiller/core/distiller_options.cc
new file mode 100644
index 0000000..6b45b611
--- /dev/null
+++ b/components/dom_distiller/core/distiller_options.cc
@@ -0,0 +1,20 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/dom_distiller/core/distiller_options.h"
+
+namespace dom_distiller {
+
+DistillerOptions::DistillerOptions() = default;
+
+DistillerOptions::DistillerOptions(
+    proto::DomDistillerOptions dom_distiller_options)
+    : dom_distiller(std::move(dom_distiller_options)) {}
+
+DistillerOptions::DistillerOptions(ReadabilityOptions readability_options)
+    : readability(std::move(readability_options)) {}
+
+DistillerOptions::~DistillerOptions() = default;
+
+}  // namespace dom_distiller
diff --git a/components/dom_distiller/core/distiller_options.h b/components/dom_distiller/core/distiller_options.h
new file mode 100644
index 0000000..781f754
--- /dev/null
+++ b/components/dom_distiller/core/distiller_options.h
@@ -0,0 +1,29 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_DOM_DISTILLER_CORE_DISTILLER_OPTIONS_H_

diff --git a/components/dom_distiller/core/distiller_page_unittest.cc b/components/dom_distiller/core/distiller_page_unittest.cc
index e7bcea0f..a7c891eb 100644
--- a/components/dom_distiller/core/distiller_page_unittest.cc
+++ b/components/dom_distiller/core/distiller_page_unittest.cc
@@ -15,6 +15,7 @@
 #include "components/dom_distiller/core/dom_distiller_constants.h"
 #include "components/dom_distiller/core/dom_distiller_features.h"
 #include "components/dom_distiller/core/extraction_utils.h"
+#include "components/dom_distiller/core/readability_options.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/dom_distiller_js/dom_distiller.pb.h"
 #include "url/gurl.h"
@@ -308,6 +309,17 @@
 }
 #endif
 
+// Test that the readability script options are injected correctly.
+TEST_F(DistillerPageTest, ReadabilityScriptOptionsHandling) {
+  ReadabilityOptions custom_options;
+  std::string default_script = GetReadabilityDistillerScript(custom_options);
+  EXPECT_NE(std::string::npos, default_script.find("})(undefined);"));
+
+  custom_options.allowed_video_regex = "youtube|vimeo";
+  std::string custom_script = GetReadabilityDistillerScript(custom_options);
+  EXPECT_NE(std::string::npos, custom_script.find("})(\"youtube|vimeo\");"));
+}
+
 }  // namespace
 
 }  // namespace dom_distiller
diff --git a/ios/chrome/browser/reader_mode/model/reader_mode_content_tab_helper_unittest.mm b/ios/chrome/browser/reader_mode/model/reader_mode_content_tab_helper_unittest.mm
index 435d322..ab94297 100644
--- a/ios/chrome/browser/reader_mode/model/reader_mode_content_tab_helper_unittest.mm
+++ b/ios/chrome/browser/reader_mode/model/reader_mode_content_tab_helper_unittest.mm
@@ -150,22 +150,29 @@
   EXPECT_NSEQ(non_content_request, delegate_->last_canceled_request());
 }
 
-// Tests that non-main frame URL requests are always allowed. This is a
-// regression test for crbug.com/426443192.
-TEST_F(ReaderModeContentTabHelperTest, AllowsContentURLRequestForNonMainFrame) {
-  NSURL* non_content_url = [NSURL URLWithString:@"https://test2.url/"];
-  NSURLRequest* non_content_request =
-      [NSURLRequest requestWithURL:non_content_url];
+// Tests that non-main frame URL requests are allowed if they are in the trusted
+// allowlist, and canceled otherwise.
+TEST_F(ReaderModeContentTabHelperTest, AllowsTrustedNonMainFrameRequestsOnly) {
+  NSURL* trusted_url =
+      [NSURL URLWithString:@"https://www.youtube.com/embed/123"];
+  NSURLRequest* trusted_request = [NSURLRequest requestWithURL:trusted_url];
   web::WebStatePolicyDecider::RequestInfo non_main_frame_request_info(
       ui::PAGE_TRANSITION_FIRST, /*target_frame_is_main=*/false, false, false,
       false, false);
 
-  // Non-main frame URLs should always be allowed.
   std::optional<web::WebStatePolicyDecider::PolicyDecision> policy_decision =
-      GetContentRequestPolicyDecision(non_content_request,
+      GetContentRequestPolicyDecision(trusted_request,
                                       non_main_frame_request_info);
   EXPECT_TRUE(policy_decision);
   EXPECT_TRUE(policy_decision->ShouldAllowNavigation());
+
+  NSURL* untrusted_url = [NSURL URLWithString:@"https://test2.url/"];
+  NSURLRequest* untrusted_request = [NSURLRequest requestWithURL:untrusted_url];
+
+  policy_decision = GetContentRequestPolicyDecision(
+      untrusted_request, non_main_frame_request_info);
+  EXPECT_TRUE(policy_decision);
+  EXPECT_TRUE(policy_decision->ShouldCancelNavigation());
 }
 
 // Tests that the delegate is notified only when the loaded page URL matches the
diff --git a/ios/chrome/browser/reader_mode/model/reader_mode_test.mm b/ios/chrome/browser/reader_mode/model/reader_mode_test.mm
index c5a9a877..d0d965e 100644
--- a/ios/chrome/browser/reader_mode/model/reader_mode_test.mm
+++ b/ios/chrome/browser/reader_mode/model/reader_mode_test.mm
@@ -16,6 +16,7 @@
 #import "components/optimization_guide/proto/hints.pb.h"
 #import "components/sync/test/test_sync_service.h"
 #import "components/translate/core/browser/translate_pref_names.h"
+#import "ios/chrome/browser/dom_distiller/model/constants.h"
 #import "ios/chrome/browser/dom_distiller/model/distiller_service_factory.h"
 #import "ios/chrome/browser/feature_engagement/model/tracker_factory.h"
 #import "ios/chrome/browser/infobars/model/infobar_manager_impl.h"
@@ -24,6 +25,7 @@
 #import "ios/chrome/browser/optimization_guide/model/optimization_guide_service.h"
 #import "ios/chrome/browser/optimization_guide/model/optimization_guide_service_factory.h"
 #import "ios/chrome/browser/overlays/model/public/overlay_request_queue.h"
+#import "ios/chrome/browser/reader_mode/model/constants.h"
 #import "ios/chrome/browser/reader_mode/model/features.h"
 #import "ios/chrome/browser/reader_mode/model/reader_mode_java_script_feature.h"
 #import "ios/chrome/browser/reader_mode/model/reader_mode_scroll_anchor_java_script_feature.h"
@@ -164,8 +166,10 @@
 
   // Set up the fake web frame to return a custom result after executing
   // the Readability Javascript.
+  dom_distiller::ReadabilityOptions options;
+  options.allowed_video_regex = kReadabilityAllowedVideoRegex;
   std::u16string readability_script =
-      base::UTF8ToUTF16(dom_distiller::GetReadabilityDistillerScript());
+      base::UTF8ToUTF16(dom_distiller::GetReadabilityDistillerScript(options));
   base::DictValue readability_result;
   readability_result.Set("content", distilled_content);
   readability_result.Set("title", "fake title");