From b709f9933538dc78cfd681bab3e09a1bc5357b20 Mon Sep 17 00:00:00 2001
From: Ming-Ying Chung <mych@chromium.org>
Date: Wed, 07 Jan 2026 00:14:27 -0800
Subject: [PATCH] [FileChooser] Block `FileChooserParams::Mode::kSave` in content

`FileChooserImpl::OpenFileChooser()` previously allowed `Mode::kSave`,
which permitted the renderer to specify a default_file_name. If the
user accepted the dialog, the renderer would be granted read/write
access to the chosen file. A compromised renderer could exploit this
by prefilling a sensitive filename and tricking the user into granting
access.

`Mode::kSave` was primarily used by the legacy PPAPI and is not used
anymore in [Blink][1]. Instead, the File System Access API handles
save pickers in the browser process, bypassing this
`blink::mojom::FileChooser` interface.

This CL explicitly blocks `Mode::kSave` and reports a bad message in
`FileChooserImpl::OpenFileChooser()` if a renderer attempts to use it.
It also removes the corresponding permission grant logic in
`FileChooserImpl::FileSelected()` and adds a safety check.

Related CL: https://crrev.com/c/6786387

[1]: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/forms/file_input_type.cc;l=210-215;drc=56535b80c32d3a618b8fc8cfd7b1afdd3862e1b2

Bug: 435684924
Change-Id: Ic696d0e7f01d1ddda1e5630c6bcb9ba889ce33ed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7400881
Commit-Queue: Ming-Ying Chung <mych@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1565463}
---

diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index eb0d950..5d81875 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -389,6 +389,30 @@
 #endif
 }
 
+// Ensure that we kill the renderer process if it tries to open a file chooser
+// with Mode::kSave, which is not allowed for renderer-initiated choosers.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest, FileChooserWithSaveMode) {
+  GURL foo("http://foo.com/simple_page.html");
+  EXPECT_TRUE(NavigateToURL(shell(), foo));
+
+  RenderFrameHost* compromised_renderer =
+      shell()->web_contents()->GetPrimaryMainFrame();
+  blink::mojom::FileChooserParamsPtr params =
+      blink::mojom::FileChooserParams::New();
+  params->mode = blink::mojom::FileChooserParams::Mode::kSave;
+  params->default_file_name = base::FilePath(FILE_PATH_LITERAL("file.txt"));
+
+  mojo::test::BadMessageObserver bad_message_observer;
+  mojo::Remote<blink::mojom::FileChooser> chooser =
+      FileChooserImpl::CreateBoundForTesting(
+          static_cast<RenderFrameHostImpl*>(compromised_renderer));
+  chooser->OpenFileChooser(
+      std::move(params), blink::mojom::FileChooser::OpenFileChooserCallback());
+  chooser.FlushForTesting();
+  EXPECT_EQ("FileChooser: Save mode is not allowed.",
+            bad_message_observer.WaitForBadMessage());
+}
+
 // A test for crbug.com/941008.
 // Calling OpenFileChooser() and EnumerateChosenDirectory() for a single
 // FileChooser instance had a problem.
diff --git a/content/browser/web_contents/file_chooser_impl.cc b/content/browser/web_contents/file_chooser_impl.cc
index 40e2ea6f..45e3a89 100644
--- a/content/browser/web_contents/file_chooser_impl.cc
+++ b/content/browser/web_contents/file_chooser_impl.cc
@@ -171,11 +171,21 @@
     return;
   }
 
+  // Do not allow save mode from the renderer process.
+  // Save mode was primarily used by PPAPI and is not used anymore in Blink,
+  // i.e. `blink::FileInputType::OpenPopupView()`.
+  // The File System Access API handles save pickers in the browser process,
+  // bypassing this Mojo interface.
+  // See https://crbug.com/435684924 for context.
+  if (params->mode == blink::mojom::FileChooserParams::Mode::kSave) {
+    mojo::ReportBadMessage("FileChooser: Save mode is not allowed.");
+    listener->FileSelectionCanceled();
+    return;
+  }
+
   // Do not allow open dialogs to have renderer-controlled default_file_name.
   // See https://crbug.com/433800617 for context.
-  if (params->mode != blink::mojom::FileChooserParams::Mode::kSave) {
-    params->default_file_name = base::FilePath();
-  }
+  params->default_file_name = base::FilePath();
 
   // Don't allow page with open FileChooser to enter BackForwardCache to avoid
   // any unexpected behaviour from BackForwardCache.
@@ -214,6 +224,13 @@
     const base::FilePath& base_dir,
     blink::mojom::FileChooserParams::Mode mode,
     std::vector<blink::mojom::FileChooserFileInfoPtr> files) {
+  if (mode == blink::mojom::FileChooserParams::Mode::kSave) {
+    // Save mode should be blocked by OpenFileChooser, but if we get here, e.g.
+    // via test, we must not process it to avoid granting read permissions to
+    // the renderer.
+    return;
+  }
+
   listener_impl_ = nullptr;
   if (!render_frame_host()) {
     std::move(callback_).Run(nullptr);
@@ -224,11 +241,6 @@
   auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
   // Grant the security access requested to the given files.
   for (const auto& file : files) {
-    if (mode == blink::mojom::FileChooserParams::Mode::kSave) {
-      policy->GrantCreateReadWriteFile(pid, file->get_native_file()->file_path);
-      continue;
-    }
-
     if (file->is_file_system()) {
       if (!file_system_context) {
         file_system_context =
diff --git a/content/browser/web_contents/file_chooser_impl_browsertest.cc b/content/browser/web_contents/file_chooser_impl_browsertest.cc
index d2891cf1..000b003 100644
--- a/content/browser/web_contents/file_chooser_impl_browsertest.cc
+++ b/content/browser/web_contents/file_chooser_impl_browsertest.cc
@@ -8,6 +8,7 @@
 #include "base/functional/bind.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
+#include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/public/common/content_paths.h"
@@ -17,6 +18,7 @@
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "content/test/content_browser_test_utils_internal.h"
+#include "third_party/blink/public/mojom/choosers/file_chooser.mojom.h"
 #include "url/gurl.h"
 #include "url/url_constants.h"
 
@@ -271,4 +273,38 @@
       EvalJs(shell(), "document.getElementById('fileinput').files[0].name;"));
 }
 
+// Ensure that FileChooserImpl::FileSelected does not grant permissions if
+// invoked with Mode::kSave, as a defense against compromised renderers.
+// See https://crbug.com/435684924.
+IN_PROC_BROWSER_TEST_F(FileChooserImplBrowserTest,
+                       FileSelectedWithSaveModeDoesNotGrantPermission) {
+  EXPECT_TRUE(NavigateToURL(shell(), GURL(url::kAboutBlankURL)));
+
+  auto* rfh = static_cast<RenderFrameHostImpl*>(
+      shell()->web_contents()->GetPrimaryMainFrame());
+  auto chooser_and_remote = FileChooserImpl::CreateForTesting(rfh);
+  auto* chooser = chooser_and_remote.first;
+
+  base::FilePath test_file;
+  EXPECT_TRUE(base::PathService::Get(base::DIR_TEMP, &test_file));
+  test_file = test_file.AppendASCII("sensitive_file.txt");
+
+  // Ensure renderer doesn't have access initially.
+  auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  EXPECT_FALSE(policy->CanReadFile(rfh->GetProcess()->GetDeprecatedID(), test_file));
+
+  std::vector<blink::mojom::FileChooserFileInfoPtr> files;
+  files.emplace_back(blink::mojom::FileChooserFileInfo::NewNativeFile(
+      blink::mojom::NativeFileInfo::New(test_file, std::u16string(),
+                                        std::vector<std::u16string>())));
+
+  // Call FileSelected with kSave.
+  chooser->FileSelected(base::FilePath(),
+                        blink::mojom::FileChooserParams::Mode::kSave,
+                        std::move(files));
+
+  // Verify renderer STILL doesn't have access.
+  EXPECT_FALSE(policy->CanReadFile(rfh->GetProcess()->GetDeprecatedID(), test_file));
+}
+
 }  // namespace content
diff --git a/content/browser/web_contents/file_chooser_impl_unittest.cc b/content/browser/web_contents/file_chooser_impl_unittest.cc
index 54d7dea..72e63b9 100644
--- a/content/browser/web_contents/file_chooser_impl_unittest.cc
+++ b/content/browser/web_contents/file_chooser_impl_unittest.cc
@@ -9,6 +9,7 @@
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/test/test_web_contents.h"
+#include "mojo/public/cpp/test_support/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -85,37 +86,24 @@
   EXPECT_EQ(captured_params->default_file_name, base::FilePath());
 }
 
-TEST_F(FileChooserImplTest, DefaultFileNamePreservedWhenModeIsSave) {

diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index eb0d950..5d81875 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -389,6 +389,30 @@
 #endif
 }
 
+// Ensure that we kill the renderer process if it tries to open a file chooser
+// with Mode::kSave, which is not allowed for renderer-initiated choosers.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest, FileChooserWithSaveMode) {
+  GURL foo("http://foo.com/simple_page.html");
+  EXPECT_TRUE(NavigateToURL(shell(), foo));
+
+  RenderFrameHost* compromised_renderer =
+      shell()->web_contents()->GetPrimaryMainFrame();
+  blink::mojom::FileChooserParamsPtr params =
+      blink::mojom::FileChooserParams::New();
+  params->mode = blink::mojom::FileChooserParams::Mode::kSave;
+  params->default_file_name = base::FilePath(FILE_PATH_LITERAL("file.txt"));
+
+  mojo::test::BadMessageObserver bad_message_observer;
+  mojo::Remote<blink::mojom::FileChooser> chooser =
+      FileChooserImpl::CreateBoundForTesting(
+          static_cast<RenderFrameHostImpl*>(compromised_renderer));
+  chooser->OpenFileChooser(
+      std::move(params), blink::mojom::FileChooser::OpenFileChooserCallback());
+  chooser.FlushForTesting();
+  EXPECT_EQ("FileChooser: Save mode is not allowed.",
+            bad_message_observer.WaitForBadMessage());
+}
+
 // A test for crbug.com/941008.
 // Calling OpenFileChooser() and EnumerateChosenDirectory() for a single
 // FileChooser instance had a problem.
diff --git a/content/browser/web_contents/file_chooser_impl_browsertest.cc b/content/browser/web_contents/file_chooser_impl_browsertest.cc
index d2891cf1..000b003 100644
--- a/content/browser/web_contents/file_chooser_impl_browsertest.cc
+++ b/content/browser/web_contents/file_chooser_impl_browsertest.cc
@@ -8,6 +8,7 @@
 #include "base/functional/bind.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
+#include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/public/common/content_paths.h"
@@ -17,6 +18,7 @@
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "content/test/content_browser_test_utils_internal.h"
+#include "third_party/blink/public/mojom/choosers/file_chooser.mojom.h"
 #include "url/gurl.h"
 #include "url/url_constants.h"
 
@@ -271,4 +273,38 @@
       EvalJs(shell(), "document.getElementById('fileinput').files[0].name;"));
 }
 
+// Ensure that FileChooserImpl::FileSelected does not grant permissions if
+// invoked with Mode::kSave, as a defense against compromised renderers.
+// See https://crbug.com/435684924.
+IN_PROC_BROWSER_TEST_F(FileChooserImplBrowserTest,
+                       FileSelectedWithSaveModeDoesNotGrantPermission) {
+  EXPECT_TRUE(NavigateToURL(shell(), GURL(url::kAboutBlankURL)));
+
+  auto* rfh = static_cast<RenderFrameHostImpl*>(
+      shell()->web_contents()->GetPrimaryMainFrame());
+  auto chooser_and_remote = FileChooserImpl::CreateForTesting(rfh);
+  auto* chooser = chooser_and_remote.first;
+
+  base::FilePath test_file;
+  EXPECT_TRUE(base::PathService::Get(base::DIR_TEMP, &test_file));
+  test_file = test_file.AppendASCII("sensitive_file.txt");
+
+  // Ensure renderer doesn't have access initially.
+  auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
+  EXPECT_FALSE(policy->CanReadFile(rfh->GetProcess()->GetDeprecatedID(), test_file));
+
+  std::vector<blink::mojom::FileChooserFileInfoPtr> files;
+  files.emplace_back(blink::mojom::FileChooserFileInfo::NewNativeFile(
+      blink::mojom::NativeFileInfo::New(test_file, std::u16string(),
+                                        std::vector<std::u16string>())));
+
+  // Call FileSelected with kSave.
+  chooser->FileSelected(base::FilePath(),
+                        blink::mojom::FileChooserParams::Mode::kSave,
+                        std::move(files));
+
+  // Verify renderer STILL doesn't have access.
+  EXPECT_FALSE(policy->CanReadFile(rfh->GetProcess()->GetDeprecatedID(), test_file));
+}
+
 }  // namespace content
diff --git a/content/browser/web_contents/file_chooser_impl_unittest.cc b/content/browser/web_contents/file_chooser_impl_unittest.cc
index 54d7dea..72e63b9 100644
--- a/content/browser/web_contents/file_chooser_impl_unittest.cc
+++ b/content/browser/web_contents/file_chooser_impl_unittest.cc
@@ -9,6 +9,7 @@
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/test/test_web_contents.h"
+#include "mojo/public/cpp/test_support/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -85,37 +86,24 @@
   EXPECT_EQ(captured_params->default_file_name, base::FilePath());
 }
 
-TEST_F(FileChooserImplTest, DefaultFileNamePreservedWhenModeIsSave) {
-  FileChooserImpl* file_chooser_impl =
-      FileChooserImpl::CreateForTesting(
-          static_cast<RenderFrameHostImpl*>(main_rfh()))
-          .first;
+TEST_F(FileChooserImplTest, SaveModeBlocked) {
+  auto chooser_and_remote = FileChooserImpl::CreateForTesting(
+      static_cast<RenderFrameHostImpl*>(main_rfh()));
+  // We keep the remote to send the message via mojo, which enables
+  // ReportBadMessage to work properly.
+  auto& chooser = chooser_and_remote.second;
 
   auto params = blink::mojom::FileChooserParams::New();
   params->mode = blink::mojom::FileChooserParams::Mode::kSave;
-  const base::FilePath kInitialFile =
-      base::FilePath(FILE_PATH_LITERAL("file.txt"));
-  params->default_file_name = kInitialFile;
+  params->default_file_name = base::FilePath(FILE_PATH_LITERAL("file.txt"));
 
-  blink::mojom::FileChooserParamsPtr captured_params;
-  EXPECT_CALL(*mock_web_contents_delegate_, RunFileChooser(_, _, _))
-      .WillOnce(
-          [&](RenderFrameHost* rfh, scoped_refptr<FileSelectListener> listener,
-              const blink::mojom::FileChooserParams& passed_params) {
-            // Capture the arguments for later inspection.
-            captured_params = passed_params.Clone();
+  EXPECT_CALL(*mock_web_contents_delegate_, RunFileChooser(_, _, _)).Times(0);
 
-            // Avoid logging error on destruction in test.
-            static_cast<FileChooserImpl::FileSelectListenerImpl*>(
-                listener.get())
-                ->SetListenerFunctionCalledTrueForTesting();
-          });
-
-  file_chooser_impl->OpenFileChooser(std::move(params), base::DoNothing());
-
-  // Verify the default file name was preserved.
-  ASSERT_TRUE(captured_params);
-  EXPECT_EQ(captured_params->default_file_name, kInitialFile);
+  mojo::test::BadMessageObserver bad_message_observer;
+  chooser->OpenFileChooser(std::move(params), base::DoNothing());
+  chooser.FlushForTesting();
+  EXPECT_EQ("FileChooser: Save mode is not allowed.",
+            bad_message_observer.WaitForBadMessage());
 }
 
 }  // namespace content