From fb23fc51367f2bc80525511d99f94d337c5c8dea Mon Sep 17 00:00:00 2001
From: Nate Chapin <japhet@chromium.org>
Date: Fri, 11 Sep 2026 17:11:16 -0700
Subject: [PATCH] Route ParseManifestFromString through PageManifestManager and ManifestManagerHost

Previously, ParseManifestFromStringJob bound
blink::mojom::ManifestManager directly from the shared WebContents'
primary main frame, bypassing ManifestManagerHost. As a result, the
parsed manifest returned by the renderer was not validated by the
browser-side security checks in MaybeGetBadMessageStringForManifest
(such as same-origin checks for file_handlers, protocol_handlers, and
share_target).

This CL routes ParseManifestFromString through PageManifestManager and
ManifestManagerHost:
- Adds ParseManifestFromString to the public
  content::PageManifestManager interface.
- Implements ParseManifestFromString in ManifestManagerHost, validating
  that the returned manifest matches manifest_url and passes
  MaybeGetBadMessageStringForManifest against document_url's origin. Any
  violation triggers a Mojo bad message and terminates the renderer.
- Refactors ParseManifestFromStringJob to use PageManifestManager
  instead of directly managing the Mojo remote.

TAG=agy
BUG=498739277
CONV=37fc643b-28bb-4f45-bc8f-25e785ccb0c6

Fixed: 498739277
Change-Id: Ia946caa285e830944116707452a34391c33e2dcf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8393837
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Nate Chapin <japhet@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1696735}
---
diff --git a/chrome/browser/web_applications/jobs/parse_manifest_from_string_job.cc b/chrome/browser/web_applications/jobs/parse_manifest_from_string_job.cc
index 37c9ca9..f984abde 100644
--- a/chrome/browser/web_applications/jobs/parse_manifest_from_string_job.cc
+++ b/chrome/browser/web_applications/jobs/parse_manifest_from_string_job.cc
@@ -13,12 +13,10 @@
 #include "base/values.h"
 #include "chrome/browser/web_applications/web_contents/web_contents_manager.h"
 #include "components/webapps/browser/web_contents/web_app_url_loader.h"
+#include "content/public/browser/page_manifest_manager.h"
 #include "content/public/browser/web_contents.h"
-#include "mojo/public/cpp/bindings/message.h"
-#include "services/service_manager/public/cpp/interface_provider.h"
 #include "third_party/blink/public/common/manifest/manifest_util.h"
 #include "third_party/blink/public/mojom/manifest/manifest.mojom.h"
-#include "third_party/blink/public/mojom/manifest/manifest_manager.mojom.h"
 #include "url/url_constants.h"
 
 namespace web_app {
@@ -64,13 +62,10 @@
   CHECK_EQ(web_contents_->GetURL(), GURL(url::kAboutBlankURL));
   debug_value_->Set("about_blank_loaded", true);
 
-  web_contents_->GetPrimaryMainFrame()->GetRemoteInterfaces()->GetInterface(
-      manifest_manager_.BindNewPipeAndPassReceiver());
-  manifest_manager_.set_disconnect_handler(
-      base::BindOnce(&ParseManifestFromStringJob::OnManifestManagerDisconnected,
-                     weak_ptr_factory_.GetWeakPtr()));
-
-  manifest_manager_->ParseManifestFromString(
+  content::PageManifestManager* page_manifest_manager =
+      content::PageManifestManager::GetOrCreate(
+          web_contents_->GetPrimaryPage());
+  page_manifest_manager->ParseManifestFromString(
       document_url_, manifest_url_, manifest_contents_,
       base::BindOnce(&ParseManifestFromStringJob::OnManifestParsed,
                      weak_ptr_factory_.GetWeakPtr()));
@@ -83,7 +78,6 @@
   // JSON and contains a valid start_url and name, installation will proceed.
   if (blink::IsEmptyManifest(manifest)) {
     debug_value_->Set("manifest_parse_error", "invalid_manifest");
-    manifest_manager_.reset();
     std::move(callback_).Run(
         base::unexpected(ParseManifestError::kEmptyOrInvalidManifest));
     return;
@@ -91,7 +85,6 @@
 
   if (!manifest->has_valid_specified_start_url) {
     debug_value_->Set("manifest_parse_error", "invalid_start_url");
-    manifest_manager_.reset();
     std::move(callback_).Run(
         base::unexpected(ParseManifestError::kStartUrlInvalid));
     return;
@@ -99,32 +92,13 @@
 
   if (!manifest->short_name.has_value() && !manifest->name.has_value()) {
     debug_value_->Set("manifest_parse_error", "missing_name");
-    manifest_manager_.reset();
     std::move(callback_).Run(
         base::unexpected(ParseManifestError::kManifestMissingNameOrShortName));
     return;
   }
 
-  if (manifest->manifest_url != manifest_url_) {
-    mojo::ReportBadMessage("Returned manifest has incorrect manifest URL");
-    manifest_manager_.reset();
-    std::move(callback_).Run(
-        base::unexpected(ParseManifestError::kInternalError));
-    return;
-  }
-  manifest_manager_.reset();
-
   debug_value_->Set("manifest_parsed", true);
   std::move(callback_).Run(std::move(manifest));
 }
 
-void ParseManifestFromStringJob::OnManifestManagerDisconnected() {
-  if (!callback_) {
-    return;
-  }
-  debug_value_->Set("manifest_parse_error", "manifest_manager_disconnected");
-  std::move(callback_).Run(
-      base::unexpected(ParseManifestError::kInternalError));
-}
-
 }  // namespace web_app
diff --git a/chrome/browser/web_applications/jobs/parse_manifest_from_string_job.h b/chrome/browser/web_applications/jobs/parse_manifest_from_string_job.h
index f35dc045..9ee69025 100644
--- a/chrome/browser/web_applications/jobs/parse_manifest_from_string_job.h
+++ b/chrome/browser/web_applications/jobs/parse_manifest_from_string_job.h
@@ -13,9 +13,7 @@
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/web_applications/model/parse_manifest_result.h"
 #include "components/webapps/browser/web_contents/web_app_url_loader.h"
-#include "mojo/public/cpp/bindings/remote.h"
 #include "third_party/blink/public/mojom/manifest/manifest.mojom-forward.h"
-#include "third_party/blink/public/mojom/manifest/manifest_manager.mojom.h"
 #include "url/gurl.h"
 
 namespace base {
@@ -59,7 +57,6 @@
  private:
   void OnAboutBlankLoaded(webapps::WebAppUrlLoaderResult result);
   void OnManifestParsed(blink::mojom::ManifestPtr manifest);
-  void OnManifestManagerDisconnected();
 
   const raw_ref<content::WebContents> web_contents_;
   GURL document_url_;
@@ -69,7 +66,6 @@
   ResultCallback callback_;
 
   std::unique_ptr<webapps::WebAppUrlLoader> url_loader_;
-  mojo::Remote<blink::mojom::ManifestManager> manifest_manager_;
 
   base::WeakPtrFactory<ParseManifestFromStringJob> weak_ptr_factory_{this};
 };
diff --git a/content/browser/manifest/manifest_manager_host.cc b/content/browser/manifest/manifest_manager_host.cc
index b03a30e..08919958 100644
--- a/content/browser/manifest/manifest_manager_host.cc
+++ b/content/browser/manifest/manifest_manager_host.cc
@@ -16,6 +16,7 @@
 #include "base/types/expected.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/public/common/content_client.h"
+#include "mojo/public/cpp/bindings/callback_helpers.h"
 #include "mojo/public/cpp/bindings/message.h"
 #include "net/base/schemeful_site.h"
 #include "services/service_manager/public/cpp/interface_provider.h"
@@ -346,6 +347,73 @@
   ValidateAndMaybeOverrideManifest(result, std::move(manifest));
 }
 
+void ManifestManagerHost::ParseManifestFromString(
+    const GURL& document_url,
+    const GURL& manifest_url,
+    const std::string& manifest_contents,
+    ParseManifestCallback callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  GetManifestManager().ParseManifestFromString(
+      document_url, manifest_url, manifest_contents,
+      mojo::WrapCallbackWithDefaultInvokeIfNotRun(
+          base::BindOnce(
+              &ManifestManagerHost::OnParseManifestFromStringResponse,
+              weak_factory_.GetWeakPtr(), document_url, manifest_url,
+              std::move(callback)),
+          blink::mojom::ManifestPtr()));
+}
+
+void ManifestManagerHost::OnParseManifestFromStringResponse(
+    const GURL& document_url,
+    const GURL& manifest_url,
+    ParseManifestCallback callback,
+    blink::mojom::ManifestPtr manifest) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  manifest = ValidateParsedManifestFromString(document_url, manifest_url,
+                                              std::move(manifest));
+  std::move(callback).Run(std::move(manifest));
+}
+
+blink::mojom::ManifestPtr ManifestManagerHost::ValidateParsedManifestFromString(
+    const GURL& document_url,
+    const GURL& manifest_url,
+    blink::mojom::ManifestPtr manifest) {
+  if (!manifest || blink::IsEmptyManifest(manifest)) {
+    return blink::mojom::Manifest::New();
+  }
+
+  if (manifest->manifest_url != manifest_url) {
+    mojo::ReportBadMessage("Returned manifest has incorrect manifest URL");
+    return blink::mojom::Manifest::New();
+  }
+
+  url::Origin document_origin = url::Origin::Create(document_url);
+  if (document_origin.opaque()) {
+    return blink::mojom::Manifest::New();
+  }
+
+  if (std::optional<std::string> bad_message_error =
+          MaybeGetBadMessageStringForManifest(
+              blink::mojom::ManifestRequestResult::kSuccess, *manifest,
+              document_origin);
+      bad_message_error.has_value()) {
+    mojo::ReportBadMessage(*bad_message_error);
+    return blink::mojom::Manifest::New();
+  }
+
+  return manifest;
+}
+
+blink::mojom::ManifestPtr
+ManifestManagerHost::ValidateParsedManifestFromStringForTesting(  // IN-TEST
+    const GURL& document_url,
+    const GURL& manifest_url,
+    blink::mojom::ManifestPtr manifest) {
+  CHECK_IS_TEST();
+  return ValidateParsedManifestFromString(document_url, manifest_url,
+                                          std::move(manifest));
+}
+
 std::vector<ManifestManagerHost::GetManifestCallback>
 ManifestManagerHost::ExtractPendingCallbacks() {
   std::vector<GetManifestCallback> callbacks;
diff --git a/content/browser/manifest/manifest_manager_host.h b/content/browser/manifest/manifest_manager_host.h
index 2c61cd88..5c41b7b 100644
--- a/content/browser/manifest/manifest_manager_host.h
+++ b/content/browser/manifest/manifest_manager_host.h
@@ -67,6 +67,16 @@
       blink::mojom::ManifestRequestResult result,
       blink::mojom::ManifestPtr manifest);
 
+  void ParseManifestFromString(const GURL& document_url,
+                               const GURL& manifest_url,
+                               const std::string& manifest_contents,
+                               ParseManifestCallback callback) override;
+
+  blink::mojom::ManifestPtr ValidateParsedManifestFromStringForTesting(
+      const GURL& document_url,
+      const GURL& manifest_url,
+      blink::mojom::ManifestPtr manifest);
+
  private:
   explicit ManifestManagerHost(Page& page);
 
@@ -81,6 +91,16 @@
       blink::mojom::ManifestRequestResult result,
       blink::mojom::ManifestPtr manifest);
 
+  blink::mojom::ManifestPtr ValidateParsedManifestFromString(
+      const GURL& document_url,
+      const GURL& manifest_url,
+      blink::mojom::ManifestPtr manifest);
+
+  void OnParseManifestFromStringResponse(const GURL& document_url,
+                                         const GURL& manifest_url,
+                                         ParseManifestCallback callback,
+                                         blink::mojom::ManifestPtr manifest);
+
   std::vector<GetManifestCallback> ExtractPendingCallbacks();
   void OnConnectionError();
 
diff --git a/content/public/browser/page_manifest_manager.h b/content/public/browser/page_manifest_manager.h
index 09b85de..01eb829 100644
--- a/content/public/browser/page_manifest_manager.h
+++ b/content/public/browser/page_manifest_manager.h
@@ -5,12 +5,16 @@
 #ifndef CONTENT_PUBLIC_BROWSER_PAGE_MANIFEST_MANAGER_H_
 #define CONTENT_PUBLIC_BROWSER_PAGE_MANIFEST_MANAGER_H_
 
+#include <string>
+
 #include "base/callback_list.h"
 #include "base/types/expected.h"
 #include "content/common/content_export.h"
 #include "third_party/blink/public/mojom/manifest/manifest.mojom-forward.h"
 #include "third_party/blink/public/mojom/manifest/manifest_manager.mojom-forward.h"
 
+class GURL;
+
 namespace content {
 class Page;
 
@@ -62,6 +66,20 @@
   // from the page (i.e. changing from a specified manifest to no manifest).
   virtual base::CallbackListSubscription GetAllSpecifiedManifests(
       AllManifestsCallbackList::CallbackType callback) = 0;
+
+  using ParseManifestCallback =
+      base::OnceCallback<void(blink::mojom::ManifestPtr)>;
+
+  // Parses the given manifest string using the renderer associated with this
+  // page. Performs browser-side security validation on the parsed manifest
+  // against `document_url` and `manifest_url`. If the renderer returns an
+  // invalid manifest (e.g. cross-origin handlers or mismatched manifest URL),
+  // a Mojo bad message is reported, terminating the renderer process, and
+  // `callback` is called with an empty ManifestPtr.
+  virtual void ParseManifestFromString(const GURL& document_url,
+                                       const GURL& manifest_url,
+                                       const std::string& manifest_contents,
+                                       ParseManifestCallback callback) = 0;
 };
 
 }  // namespace content
diff --git a/content/browser/manifest/manifest_browsertest.cc b/content/browser/manifest/manifest_browsertest.cc
index e4bf73a..5bce812 100644
--- a/content/browser/manifest/manifest_browsertest.cc
+++ b/content/browser/manifest/manifest_browsertest.cc
@@ -1761,5 +1761,94 @@
   EXPECT_EQ(u"Manifest_New", manifest.name);
 }
 
+IN_PROC_BROWSER_TEST_F(ManifestBrowserTest, ParseManifestFromString_Success) {
+  const GURL test_url =
+      embedded_test_server()->GetURL("/manifest/empty-manifest.html");
+  ASSERT_TRUE(NavigateToURL(shell(), test_url));
+
+  const GURL manifest_url =
+      embedded_test_server()->GetURL("/manifest/manifest.json");
+  const std::string manifest_contents =
+      R"({
+        "name": "Test App",
+        "start_url": "/manifest/empty-manifest.html",
+        "scope": "/manifest/"
+      })";
+
+  base::test::TestFuture<blink::mojom::ManifestPtr> manifest_future;
+  PageManifestManager* manifest_manager = PageManifestManager::GetOrCreate(
+      shell()->web_contents()->GetPrimaryPage());
+  manifest_manager->ParseManifestFromString(
+      test_url, manifest_url, manifest_contents, manifest_future.GetCallback());
+
+  blink::mojom::ManifestPtr manifest = manifest_future.Take();
+  ASSERT_TRUE(manifest);
+  EXPECT_FALSE(blink::IsEmptyManifest(manifest));
+  EXPECT_EQ(u"Test App", manifest->name);
+  EXPECT_EQ(test_url, manifest->start_url);
+}
+
+IN_PROC_BROWSER_TEST_F(
+    ManifestBrowserTest,
+    BadMessage_ParseManifestFromString_FileHandlersActionCrossOrigin) {
+  const GURL test_url =
+      embedded_test_server()->GetURL("/manifest/empty-manifest.html");
+  ASSERT_TRUE(NavigateToURL(shell(), test_url));
+
+  ManifestManagerHost* host = ManifestManagerHost::GetOrCreateForPage(
+      shell()->web_contents()->GetPrimaryPage());
+
+  mojo::FakeMessageDispatchContext fake_dispatch_context;
+  auto bad_manifest = blink::mojom::Manifest::New();
+  const GURL manifest_url =
+      embedded_test_server()->GetURL("/manifest/manifest.json");
+  bad_manifest->manifest_url = manifest_url;
+  bad_manifest->start_url = test_url;
+  bad_manifest->id = test_url;
+  bad_manifest->scope = embedded_test_server()->GetURL("/manifest/");
+
+  blink::mojom::ManifestFileHandlerPtr file_handler =
+      blink::mojom::ManifestFileHandler::New();
+  file_handler->action = GURL("https://evil.com");
+  bad_manifest->file_handlers.push_back(std::move(file_handler));
+
+  mojo::test::BadMessageObserver bad_message_observer;
+  blink::mojom::ManifestPtr result =
+      host->ValidateParsedManifestFromStringForTesting(test_url, manifest_url,
+                                                       std::move(bad_manifest));
+  EXPECT_TRUE(blink::IsEmptyManifest(result));
+  EXPECT_THAT(
+      bad_message_observer.WaitForBadMessage(),
+      ::testing::StartsWith(
+          "Manifest file_handlers must be same-origin with the document."));
+}
+
+IN_PROC_BROWSER_TEST_F(
+    ManifestBrowserTest,
+    BadMessage_ParseManifestFromString_MismatchedManifestUrl) {
+  const GURL test_url =
+      embedded_test_server()->GetURL("/manifest/empty-manifest.html");
+  ASSERT_TRUE(NavigateToURL(shell(), test_url));
+
+  ManifestManagerHost* host = ManifestManagerHost::GetOrCreateForPage(
+      shell()->web_contents()->GetPrimaryPage());
+
+  mojo::FakeMessageDispatchContext fake_dispatch_context;
+  auto bad_manifest = blink::mojom::Manifest::New();
+  bad_manifest->manifest_url = GURL("https://evil.com/manifest.json");
... (file diff truncated)

diff --git a/content/browser/manifest/manifest_browsertest.cc b/content/browser/manifest/manifest_browsertest.cc
index e4bf73a..5bce812 100644
--- a/content/browser/manifest/manifest_browsertest.cc
+++ b/content/browser/manifest/manifest_browsertest.cc
@@ -1761,5 +1761,94 @@
   EXPECT_EQ(u"Manifest_New", manifest.name);
 }
 
+IN_PROC_BROWSER_TEST_F(ManifestBrowserTest, ParseManifestFromString_Success) {
+  const GURL test_url =
+      embedded_test_server()->GetURL("/manifest/empty-manifest.html");
+  ASSERT_TRUE(NavigateToURL(shell(), test_url));
+
+  const GURL manifest_url =
+      embedded_test_server()->GetURL("/manifest/manifest.json");
+  const std::string manifest_contents =
+      R"({
+        "name": "Test App",
+        "start_url": "/manifest/empty-manifest.html",
+        "scope": "/manifest/"
+      })";
+
+  base::test::TestFuture<blink::mojom::ManifestPtr> manifest_future;
+  PageManifestManager* manifest_manager = PageManifestManager::GetOrCreate(
+      shell()->web_contents()->GetPrimaryPage());
+  manifest_manager->ParseManifestFromString(
+      test_url, manifest_url, manifest_contents, manifest_future.GetCallback());
+
+  blink::mojom::ManifestPtr manifest = manifest_future.Take();
+  ASSERT_TRUE(manifest);
+  EXPECT_FALSE(blink::IsEmptyManifest(manifest));
+  EXPECT_EQ(u"Test App", manifest->name);
+  EXPECT_EQ(test_url, manifest->start_url);
+}
+
+IN_PROC_BROWSER_TEST_F(
+    ManifestBrowserTest,
+    BadMessage_ParseManifestFromString_FileHandlersActionCrossOrigin) {
+  const GURL test_url =
+      embedded_test_server()->GetURL("/manifest/empty-manifest.html");
+  ASSERT_TRUE(NavigateToURL(shell(), test_url));
+
+  ManifestManagerHost* host = ManifestManagerHost::GetOrCreateForPage(
+      shell()->web_contents()->GetPrimaryPage());
+
+  mojo::FakeMessageDispatchContext fake_dispatch_context;
+  auto bad_manifest = blink::mojom::Manifest::New();
+  const GURL manifest_url =
+      embedded_test_server()->GetURL("/manifest/manifest.json");
+  bad_manifest->manifest_url = manifest_url;
+  bad_manifest->start_url = test_url;
+  bad_manifest->id = test_url;
+  bad_manifest->scope = embedded_test_server()->GetURL("/manifest/");
+
+  blink::mojom::ManifestFileHandlerPtr file_handler =
+      blink::mojom::ManifestFileHandler::New();
+  file_handler->action = GURL("https://evil.com");
+  bad_manifest->file_handlers.push_back(std::move(file_handler));
+
+  mojo::test::BadMessageObserver bad_message_observer;
+  blink::mojom::ManifestPtr result =
+      host->ValidateParsedManifestFromStringForTesting(test_url, manifest_url,
+                                                       std::move(bad_manifest));
+  EXPECT_TRUE(blink::IsEmptyManifest(result));
+  EXPECT_THAT(
+      bad_message_observer.WaitForBadMessage(),
+      ::testing::StartsWith(
+          "Manifest file_handlers must be same-origin with the document."));
+}
+
+IN_PROC_BROWSER_TEST_F(
+    ManifestBrowserTest,
+    BadMessage_ParseManifestFromString_MismatchedManifestUrl) {
+  const GURL test_url =
+      embedded_test_server()->GetURL("/manifest/empty-manifest.html");
+  ASSERT_TRUE(NavigateToURL(shell(), test_url));
+
+  ManifestManagerHost* host = ManifestManagerHost::GetOrCreateForPage(
+      shell()->web_contents()->GetPrimaryPage());
+
+  mojo::FakeMessageDispatchContext fake_dispatch_context;
+  auto bad_manifest = blink::mojom::Manifest::New();
+  bad_manifest->manifest_url = GURL("https://evil.com/manifest.json");
+  bad_manifest->start_url = test_url;
+  bad_manifest->id = test_url;
+  bad_manifest->scope = embedded_test_server()->GetURL("/manifest/");
+
+  mojo::test::BadMessageObserver bad_message_observer;
+  blink::mojom::ManifestPtr result =
+      host->ValidateParsedManifestFromStringForTesting(
+          test_url, embedded_test_server()->GetURL("/manifest/manifest.json"),
+          std::move(bad_manifest));
+  EXPECT_TRUE(blink::IsEmptyManifest(result));
+  EXPECT_EQ(bad_message_observer.WaitForBadMessage(),
+            "Returned manifest has incorrect manifest URL");
+}
+
 }  // namespace
 }  // namespace content