CVE-2025-12434
Overview
Background
- File System Access picker
showOpenFilePicker()/showSaveFilePicker()ask the browser process (FileSystemAccessManagerImpl) to show a nativeui::SelectFileDialogthroughFileSystemChooser, which resolves aResultCallbackwith the chosen entries.- `FileSystemChooser`
- a self-deleting
ui::SelectFileDialog::Listener.FileSelected/FileSelectionCanceledruncallback_and thendelete this. - `Visibility`
- the
WebContentsvisibility state (VISIBLE,OCCLUDED,HIDDEN). AWebContentsObserverreceives changes throughOnVisibilityChanged. - Check-to-use window
- the asynchronous gap between
ChooseEntriesvalidating the request andFileSystemChooser::CreateAndShowactually creating the dialog, plus the whole time the dialog stays open.
Root Cause Analysis
Before the patch, the only visibility gate sat in file_system_access_manager_impl.cc (hunk @@ -587, enclosing function not named; the removed ChooseEntries_HiddenTab test shows it is on the ChooseEntries path). It rejected the request with kPermissionDenied (“Tab must be visible in order to show a file picker.”) if GetVisibility() != Visibility::VISIBLE at that moment. The check ran once, before user-activation consumption and the asynchronous steps that lead to FileSystemChooser::CreateAndShow. A tab could pass the check and then be hidden before the dialog was created. Once created, the dialog also stayed up while the user switched to another tab, so a picker started by a background page could appear over, and seem to belong to, a different foreground tab. That is the “Race in Storage”.
The fix removes the early check and enforces visibility where it matters. CreateAndShow now aborts with kOperationAborted if GetVisibility() == Visibility::HIDDEN immediately before creating the dialog. FileSystemChooser also becomes a WebContentsObserver, and its new OnVisibilityChanged calls FileSelectionCanceled() when the tab becomes HIDDEN, closing the dialog. The new condition is narrower than the old one. The old check rejected anything not VISIBLE (including OCCLUDED), while the new checks reject only HIDDEN, so an occluded tab may now open a picker.
The fix adds continuous enforcement instead of tightening every point.
Attack Path
- Request a picker
A page with user activation calls
showOpenFilePicker(), and the manager-side visibility check passes because the tab isVISIBLE. - Change visibility
Before or after the dialog is created, the tab becomes hidden, for example because a new foreground tab opens, as in the
ShowOpenFileThenHideregression test (NEW_FOREGROUND_TAB). - Dialog persists unattributed (pre-patch)
FileSystemChooserhas no visibility observer, so the native picker stays open over a different tab, and the user may pick a file or directory thinking another site asked for it (inferred social-engineering outcome). - Grant to background page
The selection resolves the hidden page’s
ResultCallback, giving it aFileSystemHandleto the chosen entry (inferred; the diff shows the dialog surviving the hide, not a specific misuse).
Impact Assessment
(inferred).Changed Functions
| Function | Change | Notes |
|---|---|---|
ifcontent/browser/file_system_access/file_system_chooser.cc |
modified | |
CONTENT_EXPORTcontent/browser/file_system_access/file_system_chooser.h |
modified | |
IN_PROC_BROWSER_TEST_Pchrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc |
modified | |
ASSERT_TRUEchrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc |
modified | |
FileSystemAccessBrowserTestForWebUIchrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc |
modified | |
TEST_Fcontent/browser/file_system_access/file_system_access_manager_impl_unittest.cc |
modified | |
IN_PROC_BROWSER_TEST_Fcontent/browser/file_system_access/file_system_chooser_browsertest.cc |
modified | |
ASSERT_TRUEcontent/browser/file_system_access/file_system_chooser_browsertest.cc |
modified | |
FileSystemChooserBackForwardCacheBrowserTestcontent/browser/file_system_access/file_system_chooser_browsertest.cc |
modified | |
FileSystemChooserTestcontent/browser/file_system_access/file_system_chooser_unittest.cc |
modified |
Files Changed
chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cccontent/browser/file_system_access/file_system_access_manager_impl.cccontent/browser/file_system_access/file_system_access_manager_impl_unittest.cccontent/browser/file_system_access/file_system_chooser.cccontent/browser/file_system_access/file_system_chooser.hcontent/browser/file_system_access/file_system_chooser_browsertest.cccontent/browser/file_system_access/file_system_chooser_unittest.cccontent/public/test/file_system_chooser_test_helpers.cccontent/public/test/file_system_chooser_test_helpers.h
Audit Directions
- One-shot visibility/focus gates before async UILook for other browser-side prompts (permission bubbles,
showDirectoryPicker, WebAuthn, payment,<input type=file>choosers, download prompts) that checkGetVisibility()or focus once and then post tasks before showing UI. Each should re-check at show time and observe changes while the UI is open. - `OCCLUDED` vs `HIDDEN` semanticsThe new checks reject only
HIDDEN, while the old one rejected anything notVISIBLE. Assess whether occluded or partially covered tabs, or other windows in front, still let a misattributed dialog appear. - Self-deleting listeners driven by observers
OnVisibilityChanged→FileSelectionCanceled→delete thisruns inside aWebContentsObservernotification. Check that~FileSystemChooserdetachesdialog_(for example withListenerDestroyed) so a lateFileSelectedfrom the native dialog cannot reach a freed listener, and audit similar listener-plus-observer classes for double-callback or UAF.
Patch
diff --git a/content/browser/file_system_access/file_system_access_manager_impl.cc b/content/browser/file_system_access/file_system_access_manager_impl.cc
index 8debb42..39aa8aa 100644
--- a/content/browser/file_system_access/file_system_access_manager_impl.cc
+++ b/content/browser/file_system_access/file_system_access_manager_impl.cc
@@ -587,15 +587,6 @@
return;
}
- if (web_contents->GetVisibility() != Visibility::VISIBLE) {
- std::move(callback).Run(
- file_system_access_error::FromStatus(
- FileSystemAccessStatus::kPermissionDenied,
- "Tab must be visible in order to show a file picker."),
- std::vector<blink::mojom::FileSystemAccessEntryPtr>());
- return;
- }
-
// Consume user activation to address this issue: crbug.com/40059071
// TODO(crbug.com/411125804): Consider moving this user activation check to
// the renderer process or informing the renderer that it lost user
diff --git a/content/browser/file_system_access/file_system_chooser.cc b/content/browser/file_system_access/file_system_chooser.cc
index d510a87..e240d01 100644
--- a/content/browser/file_system_access/file_system_chooser.cc
+++ b/content/browser/file_system_access/file_system_chooser.cc
@@ -19,6 +19,7 @@
#include "content/browser/file_system_access/file_system_access_error.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/visibility.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_client.h"
#include "net/base/mime_util.h"
@@ -293,6 +294,16 @@
: fullscreen_block(std::move(fullscreen_block)),
pip_tucker(std::move(pip_tucker)) {}
+namespace {
+// Called when no file is selected due to being aborted.
+void AbortedCallback(FileSystemChooser::ResultCallback callback) {
+ std::move(callback).Run(
+ file_system_access_error::FromStatus(
+ blink::mojom::FileSystemAccessStatus::kOperationAborted),
+ {});
+}
+} // namespace
+
// static
void FileSystemChooser::CreateAndShow(
WebContents* web_contents,
@@ -301,9 +312,14 @@
FileSystemChooser::ScopedObjects scoped_objects) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
TRACE_EVENT0("FileSystem", "FileSystemChooser::CreateAndShow");
+ if (web_contents->GetVisibility() == Visibility::HIDDEN) {
+ AbortedCallback(std::move(callback));
+ return;
+ }
// `listener` deletes itself.
- auto* listener = new FileSystemChooser(options.type(), std::move(callback),
- std::move(scoped_objects));
+ auto* listener =
+ new FileSystemChooser(options.type(), std::move(callback),
+ std::move(scoped_objects), web_contents);
listener->dialog_ = ui::SelectFileDialog::Create(
listener,
GetContentClient()->browser()->CreateSelectFilePolicy(web_contents));
@@ -369,8 +385,10 @@
FileSystemChooser::FileSystemChooser(
ui::SelectFileDialog::Type type,
ResultCallback callback,
- FileSystemChooser::ScopedObjects scoped_objects)
- : type_(type),
+ FileSystemChooser::ScopedObjects scoped_objects,
+ WebContents* web_contents)
+ : WebContentsObserver(web_contents),
+ type_(type),
callback_(std::move(callback)),
scoped_objects_(std::move(scoped_objects)) {
CHECK(IsValidFileDialogType(type_));
@@ -383,6 +401,13 @@
}
}
+void FileSystemChooser::OnVisibilityChanged(Visibility visibility) {
+ DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+ if (visibility == Visibility::HIDDEN) {
+ FileSelectionCanceled();
+ }
+}
+
void FileSystemChooser::FileSelected(const ui::SelectedFileInfo& file,
int index) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -405,10 +430,7 @@
void FileSystemChooser::FileSelectionCanceled() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- std::move(callback_).Run(
- file_system_access_error::FromStatus(
- blink::mojom::FileSystemAccessStatus::kOperationAborted),
- {});
+ AbortedCallback(std::move(callback_));
delete this;
}
diff --git a/content/browser/file_system_access/file_system_chooser.h b/content/browser/file_system_access/file_system_chooser.h
index 440e0d0f..ee18ed9 100644
--- a/content/browser/file_system_access/file_system_chooser.h
+++ b/content/browser/file_system_access/file_system_chooser.h
@@ -16,6 +16,7 @@
#include "build/build_config.h"
#include "content/common/content_export.h"
#include "content/public/browser/file_system_access_permission_context.h"
+#include "content/public/browser/web_contents_observer.h"
#include "third_party/blink/public/mojom/file_system_access/file_system_access_manager.mojom.h"
#include "ui/shell_dialogs/select_file_dialog.h"
@@ -28,7 +29,8 @@
// a callback on a specific task runner. Furthermore the listener will delete
// itself when any of its listener methods are called.
// All of this class has to be called on the UI thread.
-class CONTENT_EXPORT FileSystemChooser : public ui::SelectFileDialog::Listener {
+class CONTENT_EXPORT FileSystemChooser : public ui::SelectFileDialog::Listener,
+ WebContentsObserver {
public:
using ResultCallback =
base::OnceCallback<void(blink::mojom::FileSystemAccessErrorPtr,
@@ -103,7 +105,8 @@
FileSystemChooser(ui::SelectFileDialog::Type type,
ResultCallback callback,
- ScopedObjects scoped_objects);
+ ScopedObjects scoped_objects,
+ WebContents* web_contents);
private:
~FileSystemChooser() override;
@@ -114,6 +117,8 @@
const std::vector<ui::SelectedFileInfo>& files) override;
void FileSelectionCanceled() override;
+ // WebContentsObserver
+ void OnVisibilityChanged(Visibility visibility) override;
SEQUENCE_CHECKER(sequence_checker_);
const ui::SelectFileDialog::Type type_;
diff --git a/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc b/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc
index d8073d33..3f97c77 100644
--- a/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc
+++ b/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc
@@ -10,6 +10,7 @@
#include "base/path_service.h"
#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h"
+#include "base/task/current_thread.h"
#include "base/test/bind.h"
#include "base/test/scoped_path_override.h"
#include "base/test/test_file_util.h"
@@ -46,6 +47,7 @@
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/fenced_frame_test_util.h"
+#include "content/public/test/file_system_chooser_test_helpers.h"
#include "content/public/test/prerender_test_util.h"
#include "content/public/test/scoped_web_ui_controller_factory_registration.h"
#include "content/public/test/test_navigation_observer.h"
@@ -1322,11 +1324,44 @@
// Try to show a file picker in the background tab.
// This should be blocked.
- EXPECT_EQ("NotAllowedError",
+ EXPECT_EQ("AbortError",
content::EvalJs(second_tab,
"self.showOpenFilePicker().catch(e => e.name)"));
}
+// Test that opening another tab while the dialog is showing closes the dialog.
+// https://crbug.com/419721056
+IN_PROC_BROWSER_TEST_P(FileSystemAccessBrowserTest, ShowOpenFileThenHide) {
+ ASSERT_TRUE(ui_test_utils::NavigateToURL(
+ browser(), embedded_test_server()->GetURL("/title1.html")));
+ content::WebContents* first_tab =
+ browser()->tab_strip_model()->GetActiveWebContents();
+
+ // Open the dialog and wait until it's created.
+ content::SelectFileDialogRecorder recorder;
+ ui::SelectFileDialog::SetFactory(
+ std::make_unique<content::ObservableSelectFileDialogFactory>(&recorder));
+ ASSERT_EQ(42,
+ content::EvalJs(
+ first_tab,
+ "window.p = self.showOpenFilePicker().catch(e => e.name); 42"));
+ ASSERT_TRUE(base::test::RunUntil([&recorder]() {
+ return recorder.state != content::SelectFileDialogRecorder::kNotCreated;
+ }));
+
+ // Create a second tab.
+ ui_test_utils::NavigateToURLWithDisposition(
+ browser(), embedded_test_server()->GetURL("/title2.html"),
+ WindowOpenDisposition::NEW_FOREGROUND_TAB,
+ ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP);
+
+ // The first tab should not be the active tab anymore.
+ ASSERT_NE(first_tab, browser()->tab_strip_model()->GetActiveWebContents());
+
+ // Check that the dialog was closed.
+ ASSERT_EQ("AbortError", content::EvalJs(first_tab, "window.p"));
+}
+
class FileSystemAccessBrowserTestForWebUI
: public InProcessBrowserTest,
public ::testing::WithParamInterface<bool> {
diff --git a/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc b/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
index 045041db..c2d849d2 100644
--- a/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
+++ b/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
@@ -2109,61 +2109,6 @@
bad_message_observer.WaitForBadMessage());
}
-TEST_F(FileSystemAccessManagerImplTest, ChooseEntries_HiddenTab) {
- base::FilePath test_file = dir_.GetPath().AppendASCII("asdf");
- ASSERT_TRUE(base::CreateTemporaryFile(&test_file));
- PathInfo test_file_info(test_file);
-
- manager_->SetFilePickerResultForTesting(test_file_info);
-
- static_cast<TestRenderFrameHost*>(web_contents_->GetPrimaryMainFrame())
- ->SimulateUserActivation();
-
- mojo::Remote<blink::mojom::FileSystemAccessManager> manager_remote;
- FileSystemAccessManagerImpl::BindingContext binding_context = {
- kTestStorageKey, kTestURL,
- web_contents_->GetPrimaryMainFrame()->GetGlobalId()};
- manager_->BindReceiver(binding_context,
- manager_remote.BindNewPipeAndPassReceiver());
-
- EXPECT_CALL(permission_context_,
- CanObtainReadPermission(kTestStorageKey.origin()))
- .WillOnce(testing::Return(true));
- EXPECT_CALL(permission_context_,
- CanObtainWritePermission(kTestStorageKey.origin()))
- .WillOnce(testing::Return(true));
- EXPECT_CALL(permission_context_, CheckPathsAgainstEnterprisePolicy(
- testing::_, testing::_, testing::_))
- .Times(0);
-
- // Hiding the web contents should make ChooseEntries return an error.
- web_contents_->WasHidden();
-
- auto save_file_picker_options = blink::mojom::SaveFilePickerOptions::New(
- blink::mojom::AcceptsTypesInfo::New(
- std::vector<blink::mojom::ChooseFileSystemEntryAcceptsOptionPtr>(),
- /*include_accepts_all=*/true),
- /*suggested_name=*/std::string());
- auto picker_options = blink::mojom::FilePickerOptions::New(
- blink::mojom::TypeSpecificFilePickerOptionsUnion::
- NewSaveFilePickerOptions(std::move(save_file_picker_options)),
- /*starting_directory_id=*/std::string(),
- blink::mojom::FilePickerStartInOptionsUnionPtr());
-
- base::test::TestFuture<blink::mojom::FileSystemAccessErrorPtr,
- std::vector<blink::mojom::FileSystemAccessEntryPtr>>
- future;
- manager_remote->ChooseEntries(std::move(picker_options),
- future.GetCallback());
- ASSERT_TRUE(future.Wait());
-
- EXPECT_EQ(future.Get<0>()->status,
- blink::mojom::FileSystemAccessStatus::kPermissionDenied);
- EXPECT_EQ(future.Get<0>()->message,
- "Tab must be visible in order to show a file picker.");
- EXPECT_EQ(future.Get<1>().size(), 0);
-}
-
TEST_F(FileSystemAccessManagerImplTest, GetUniqueId) {
const PathInfo kTestPathInfo(dir_.GetPath().AppendASCII("foo"));
ASSERT_OK_AND_ASSIGN(auto default_bucket,
diff --git a/content/browser/file_system_access/file_system_chooser_browsertest.cc b/content/browser/file_system_access/file_system_chooser_browsertest.cc
index 4248b0a3..1178ae5 100644
--- a/content/browser/file_system_access/file_system_chooser_browsertest.cc
+++ b/content/browser/file_system_access/file_system_chooser_browsertest.cc
@@ -11,6 +11,7 @@
#include "base/files/scoped_temp_dir.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/gmock_callback_support.h"
+#include "base/test/run_until.h"
#include "base/test/scoped_feature_list.h"
#include "base/threading/thread_restrictions.h"
#include "build/build_config.h"
@@ -2159,6 +2160,69 @@
dialog_params_.title);
}
+// Hide the WebContents and ensure the dialog is not shown.
+IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest, DontShowWhileHidden) {
+ FakeFileSystemAccessPermissionContext permission_context;
+ static_cast<FileSystemAccessManagerImpl*>(
+ shell()
+ ->web_contents()
+ ->GetBrowserContext()
+ ->GetStoragePartition(shell()->web_contents()->GetSiteInstance())
+ ->GetFileSystemAccessEntryFactory())
+ ->SetPermissionContextForTesting(&permission_context);
+
+ GURL url = embedded_test_server()->GetURL("/title1.html");
+ ASSERT_TRUE(NavigateToURL(shell(), url));
+
+ // Record the state of the dialog.
+ SelectFileDialogRecorder recorder;
+ ui::SelectFileDialog::SetFactory(
+ std::make_unique<ObservableSelectFileDialogFactory>(&recorder));
+
+ // Hide the WebContents.
+ WebContents* wc = shell()->web_contents();
+ wc->UpdateWebContentsVisibility(content::Visibility::HIDDEN);
+
+ // JS should see the dialog as aborted.
+ EXPECT_EQ(
+ "AbortError",
+ content::EvalJs(wc, "window.showOpenFilePicker().catch(e => e.name)"));
+ // The dialog should not have been created.
+ EXPECT_EQ(recorder.state, SelectFileDialogRecorder::kNotCreated);
+}
+
+// Show the dialog then hide the WebContents and ensure the dialog is dismissed.
+IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest, ShowThenHide) {
+ GURL url = embedded_test_server()->GetURL("/title1.html");
+ ASSERT_TRUE(NavigateToURL(shell(), url));
+
+ WebContents* wc = shell()->web_contents();
+
+ // Record the state of the dialog.
+ SelectFileDialogRecorder recorder;
+ ui::SelectFileDialog::SetFactory(
+ std::make_unique<ObservableSelectFileDialogFactory>(&recorder));
+
+ // Open the dialog and wait until it's created.
+ ASSERT_EQ(
+ 42,
+ content::EvalJs(
+ wc, "window.p = self.showOpenFilePicker().catch(e => e.name); 42"));
+ ASSERT_TRUE(base::test::RunUntil([&recorder]() {
+ return recorder.state != SelectFileDialogRecorder::kNotCreated;
+ }));
+
+ // Hide the WebContents.
+ wc->UpdateWebContentsVisibility(
+ content::Visibility::HIDDEN); // Hide the web contents.
+ ASSERT_TRUE(base::test::RunUntil([&recorder]() {
+ return recorder.state == SelectFileDialogRecorder::kDestroyed;
+ }));
+
+ // JS should see the dialog as aborted.
+ EXPECT_EQ("AbortError", content::EvalJs(wc, "p"));
+}
+
class FileSystemChooserBackForwardCacheBrowserTest
: public FileSystemChooserBrowserTest {
public:
diff --git a/content/browser/file_system_access/file_system_chooser_unittest.cc b/content/browser/file_system_access/file_system_chooser_unittest.cc
index 1fa969e..a360293 100644
--- a/content/browser/file_system_access/file_system_chooser_unittest.cc
+++ b/content/browser/file_system_access/file_system_chooser_unittest.cc
@@ -30,13 +30,20 @@
class FileSystemChooserTest : public RenderViewHostImplTestHarness {
public:
- void TearDown() override {
- RenderViewHostImplTestHarness::TearDown();
- ui::SelectFileDialog::SetFactory(nullptr);
+ void SetUp() override {
+ RenderViewHostImplTestHarness::SetUp();
+ web_contents_ = CreateTestWebContents(GetBrowserContext());
}
+ void TearDown() override {
+ ui::SelectFileDialog::SetFactory(nullptr);
+ web_contents_.reset();
+ RenderViewHostImplTestHarness::TearDown();
+ }
+
+ WebContents* web_contents() { return web_contents_.get(); }
+
std::vector<PathInfo> SyncShowDialog(
- WebContents* web_contents,
std::vector<blink::mojom::ChooseFileSystemEntryAcceptsOptionPtr> accepts,
bool include_accepts_all,
base::FilePath default_directory = base::FilePath(),
@@ -45,7 +52,7 @@
std::vector<PathInfo>>
future;
FileSystemChooser::CreateAndShow(
- web_contents,
+ web_contents(),
FileSystemChooser::Options(ui::SelectFileDialog::SELECT_OPEN_FILE,
blink::mojom::AcceptsTypesInfo::New(
std::move(accepts), include_accepts_all),
@@ -63,6 +70,7 @@
browser_context, std::move(site_instance));
... (file diff truncated)
Regression Test / PoC
diff --git a/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc b/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc
index d8073d33..3f97c77 100644
--- a/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc
+++ b/chrome/browser/ui/views/file_system_access/file_system_access_browsertest.cc
@@ -10,6 +10,7 @@
#include "base/path_service.h"
#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h"
+#include "base/task/current_thread.h"
#include "base/test/bind.h"
#include "base/test/scoped_path_override.h"
#include "base/test/test_file_util.h"
@@ -46,6 +47,7 @@
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/fenced_frame_test_util.h"
+#include "content/public/test/file_system_chooser_test_helpers.h"
#include "content/public/test/prerender_test_util.h"
#include "content/public/test/scoped_web_ui_controller_factory_registration.h"
#include "content/public/test/test_navigation_observer.h"
@@ -1322,11 +1324,44 @@
// Try to show a file picker in the background tab.
// This should be blocked.
- EXPECT_EQ("NotAllowedError",
+ EXPECT_EQ("AbortError",
content::EvalJs(second_tab,
"self.showOpenFilePicker().catch(e => e.name)"));
}
+// Test that opening another tab while the dialog is showing closes the dialog.
+// https://crbug.com/419721056
+IN_PROC_BROWSER_TEST_P(FileSystemAccessBrowserTest, ShowOpenFileThenHide) {
+ ASSERT_TRUE(ui_test_utils::NavigateToURL(
+ browser(), embedded_test_server()->GetURL("/title1.html")));
+ content::WebContents* first_tab =
+ browser()->tab_strip_model()->GetActiveWebContents();
+
+ // Open the dialog and wait until it's created.
+ content::SelectFileDialogRecorder recorder;
+ ui::SelectFileDialog::SetFactory(
+ std::make_unique<content::ObservableSelectFileDialogFactory>(&recorder));
+ ASSERT_EQ(42,
+ content::EvalJs(
+ first_tab,
+ "window.p = self.showOpenFilePicker().catch(e => e.name); 42"));
+ ASSERT_TRUE(base::test::RunUntil([&recorder]() {
+ return recorder.state != content::SelectFileDialogRecorder::kNotCreated;
+ }));
+
+ // Create a second tab.
+ ui_test_utils::NavigateToURLWithDisposition(
+ browser(), embedded_test_server()->GetURL("/title2.html"),
+ WindowOpenDisposition::NEW_FOREGROUND_TAB,
+ ui_test_utils::BROWSER_TEST_WAIT_FOR_LOAD_STOP);
+
+ // The first tab should not be the active tab anymore.
+ ASSERT_NE(first_tab, browser()->tab_strip_model()->GetActiveWebContents());
+
+ // Check that the dialog was closed.
+ ASSERT_EQ("AbortError", content::EvalJs(first_tab, "window.p"));
+}
+
class FileSystemAccessBrowserTestForWebUI
: public InProcessBrowserTest,
public ::testing::WithParamInterface<bool> {
diff --git a/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc b/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
index 045041db..c2d849d2 100644
--- a/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
+++ b/content/browser/file_system_access/file_system_access_manager_impl_unittest.cc
@@ -2109,61 +2109,6 @@
bad_message_observer.WaitForBadMessage());
}
-TEST_F(FileSystemAccessManagerImplTest, ChooseEntries_HiddenTab) {
- base::FilePath test_file = dir_.GetPath().AppendASCII("asdf");
- ASSERT_TRUE(base::CreateTemporaryFile(&test_file));
- PathInfo test_file_info(test_file);
-
- manager_->SetFilePickerResultForTesting(test_file_info);
-
- static_cast<TestRenderFrameHost*>(web_contents_->GetPrimaryMainFrame())
- ->SimulateUserActivation();
-
- mojo::Remote<blink::mojom::FileSystemAccessManager> manager_remote;
- FileSystemAccessManagerImpl::BindingContext binding_context = {
- kTestStorageKey, kTestURL,
- web_contents_->GetPrimaryMainFrame()->GetGlobalId()};
- manager_->BindReceiver(binding_context,
- manager_remote.BindNewPipeAndPassReceiver());
-
- EXPECT_CALL(permission_context_,
- CanObtainReadPermission(kTestStorageKey.origin()))
- .WillOnce(testing::Return(true));
- EXPECT_CALL(permission_context_,
- CanObtainWritePermission(kTestStorageKey.origin()))
- .WillOnce(testing::Return(true));
- EXPECT_CALL(permission_context_, CheckPathsAgainstEnterprisePolicy(
- testing::_, testing::_, testing::_))
- .Times(0);
-
- // Hiding the web contents should make ChooseEntries return an error.
- web_contents_->WasHidden();
-
- auto save_file_picker_options = blink::mojom::SaveFilePickerOptions::New(
- blink::mojom::AcceptsTypesInfo::New(
- std::vector<blink::mojom::ChooseFileSystemEntryAcceptsOptionPtr>(),
- /*include_accepts_all=*/true),
- /*suggested_name=*/std::string());
- auto picker_options = blink::mojom::FilePickerOptions::New(
- blink::mojom::TypeSpecificFilePickerOptionsUnion::
- NewSaveFilePickerOptions(std::move(save_file_picker_options)),
- /*starting_directory_id=*/std::string(),
- blink::mojom::FilePickerStartInOptionsUnionPtr());
-
- base::test::TestFuture<blink::mojom::FileSystemAccessErrorPtr,
- std::vector<blink::mojom::FileSystemAccessEntryPtr>>
- future;
- manager_remote->ChooseEntries(std::move(picker_options),
- future.GetCallback());
- ASSERT_TRUE(future.Wait());
-
- EXPECT_EQ(future.Get<0>()->status,
- blink::mojom::FileSystemAccessStatus::kPermissionDenied);
- EXPECT_EQ(future.Get<0>()->message,
- "Tab must be visible in order to show a file picker.");
- EXPECT_EQ(future.Get<1>().size(), 0);
-}
-
TEST_F(FileSystemAccessManagerImplTest, GetUniqueId) {
const PathInfo kTestPathInfo(dir_.GetPath().AppendASCII("foo"));
ASSERT_OK_AND_ASSIGN(auto default_bucket,
diff --git a/content/browser/file_system_access/file_system_chooser_browsertest.cc b/content/browser/file_system_access/file_system_chooser_browsertest.cc
index 4248b0a3..1178ae5 100644
--- a/content/browser/file_system_access/file_system_chooser_browsertest.cc
+++ b/content/browser/file_system_access/file_system_chooser_browsertest.cc
@@ -11,6 +11,7 @@
#include "base/files/scoped_temp_dir.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/gmock_callback_support.h"
+#include "base/test/run_until.h"
#include "base/test/scoped_feature_list.h"
#include "base/threading/thread_restrictions.h"
#include "build/build_config.h"
@@ -2159,6 +2160,69 @@
dialog_params_.title);
}
+// Hide the WebContents and ensure the dialog is not shown.
+IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest, DontShowWhileHidden) {
+ FakeFileSystemAccessPermissionContext permission_context;
+ static_cast<FileSystemAccessManagerImpl*>(
+ shell()
+ ->web_contents()
+ ->GetBrowserContext()
+ ->GetStoragePartition(shell()->web_contents()->GetSiteInstance())
+ ->GetFileSystemAccessEntryFactory())
+ ->SetPermissionContextForTesting(&permission_context);
+
+ GURL url = embedded_test_server()->GetURL("/title1.html");
+ ASSERT_TRUE(NavigateToURL(shell(), url));
+
+ // Record the state of the dialog.
+ SelectFileDialogRecorder recorder;
+ ui::SelectFileDialog::SetFactory(
+ std::make_unique<ObservableSelectFileDialogFactory>(&recorder));
+
+ // Hide the WebContents.
+ WebContents* wc = shell()->web_contents();
+ wc->UpdateWebContentsVisibility(content::Visibility::HIDDEN);
+
+ // JS should see the dialog as aborted.
+ EXPECT_EQ(
+ "AbortError",
+ content::EvalJs(wc, "window.showOpenFilePicker().catch(e => e.name)"));
+ // The dialog should not have been created.
+ EXPECT_EQ(recorder.state, SelectFileDialogRecorder::kNotCreated);
+}
+
+// Show the dialog then hide the WebContents and ensure the dialog is dismissed.
+IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest, ShowThenHide) {
+ GURL url = embedded_test_server()->GetURL("/title1.html");
+ ASSERT_TRUE(NavigateToURL(shell(), url));
+
+ WebContents* wc = shell()->web_contents();
+
+ // Record the state of the dialog.
+ SelectFileDialogRecorder recorder;
+ ui::SelectFileDialog::SetFactory(
+ std::make_unique<ObservableSelectFileDialogFactory>(&recorder));
+
+ // Open the dialog and wait until it's created.
+ ASSERT_EQ(
+ 42,
+ content::EvalJs(
+ wc, "window.p = self.showOpenFilePicker().catch(e => e.name); 42"));
+ ASSERT_TRUE(base::test::RunUntil([&recorder]() {
+ return recorder.state != SelectFileDialogRecorder::kNotCreated;
+ }));
+
+ // Hide the WebContents.
+ wc->UpdateWebContentsVisibility(
+ content::Visibility::HIDDEN); // Hide the web contents.
+ ASSERT_TRUE(base::test::RunUntil([&recorder]() {
+ return recorder.state == SelectFileDialogRecorder::kDestroyed;
+ }));
+
+ // JS should see the dialog as aborted.
+ EXPECT_EQ("AbortError", content::EvalJs(wc, "p"));
+}
+
class FileSystemChooserBackForwardCacheBrowserTest
: public FileSystemChooserBrowserTest {
public:
diff --git a/content/browser/file_system_access/file_system_chooser_unittest.cc b/content/browser/file_system_access/file_system_chooser_unittest.cc
index 1fa969e..a360293 100644
--- a/content/browser/file_system_access/file_system_chooser_unittest.cc
+++ b/content/browser/file_system_access/file_system_chooser_unittest.cc
@@ -30,13 +30,20 @@
class FileSystemChooserTest : public RenderViewHostImplTestHarness {
public:
- void TearDown() override {
- RenderViewHostImplTestHarness::TearDown();
- ui::SelectFileDialog::SetFactory(nullptr);
+ void SetUp() override {
+ RenderViewHostImplTestHarness::SetUp();
+ web_contents_ = CreateTestWebContents(GetBrowserContext());
}
+ void TearDown() override {
+ ui::SelectFileDialog::SetFactory(nullptr);
+ web_contents_.reset();
+ RenderViewHostImplTestHarness::TearDown();
+ }
+
+ WebContents* web_contents() { return web_contents_.get(); }
+
std::vector<PathInfo> SyncShowDialog(
- WebContents* web_contents,
std::vector<blink::mojom::ChooseFileSystemEntryAcceptsOptionPtr> accepts,
bool include_accepts_all,
base::FilePath default_directory = base::FilePath(),
@@ -45,7 +52,7 @@
std::vector<PathInfo>>
future;
FileSystemChooser::CreateAndShow(
- web_contents,
+ web_contents(),
FileSystemChooser::Options(ui::SelectFileDialog::SELECT_OPEN_FILE,
blink::mojom::AcceptsTypesInfo::New(
std::move(accepts), include_accepts_all),
@@ -63,6 +70,7 @@
browser_context, std::move(site_instance));
}
+ std::unique_ptr<content::WebContents> web_contents_;
// Must persist throughout TearDown().
SelectFileDialogParams dialog_params_;
};
@@ -70,7 +78,7 @@
TEST_F(FileSystemChooserTest, EmptyAccepts) {
ui::SelectFileDialog::SetFactory(
std::make_unique<CancellingSelectFileDialogFactory>(&dialog_params_));
- SyncShowDialog(/*web_contents=*/nullptr, {}, /*include_accepts_all=*/true);
+ SyncShowDialog({}, /*include_accepts_all=*/true);
ASSERT_TRUE(dialog_params_.file_types);
EXPECT_TRUE(dialog_params_.file_types->include_all_files);
@@ -86,7 +94,7 @@
TEST_F(FileSystemChooserTest, EmptyAcceptsIgnoresIncludeAcceptsAll) {
ui::SelectFileDialog::SetFactory(
std::make_unique<CancellingSelectFileDialogFactory>(&dialog_params_));
- SyncShowDialog(/*web_contents=*/nullptr, {}, /*include_accepts_all=*/false);
+ SyncShowDialog({}, /*include_accepts_all=*/false);
// Should still include_all_files, even though include_accepts_all was false.
ASSERT_TRUE(dialog_params_.file_types);
@@ -110,7 +118,7 @@
accepts.emplace_back(blink::mojom::ChooseFileSystemEntryAcceptsOption::New(
u"Images", std::vector<std::string>({"image/*"}),
std::vector<std::string>({})));
- SyncShowDialog(/*web_contents=*/nullptr, std::move(accepts),
+ SyncShowDialog(std::move(accepts),
/*include_accepts_all=*/true);
ASSERT_TRUE(dialog_params_.file_types);
@@ -152,7 +160,7 @@
accepts.emplace_back(blink::mojom::ChooseFileSystemEntryAcceptsOption::New(
u"", std::vector<std::string>({}),
std::vector<std::string>({"text", "js", "text"})));
- SyncShowDialog(/*web_contents=*/nullptr, std::move(accepts),
+ SyncShowDialog(std::move(accepts),
/*include_accepts_all=*/true);
ASSERT_TRUE(dialog_params_.file_types);
@@ -182,7 +190,7 @@
accepts.emplace_back(blink::mojom::ChooseFileSystemEntryAcceptsOption::New(
u"", std::vector<std::string>({"image/*"}),
std::vector<std::string>({"text", "jpg"})));
... (truncated)
Original Bug Report
Cross origin dialog spoof
Steps to reproduce the problem
showOpenFilePicker dialog can be shown over a cross-origin page which might confuse a user into leaking sensitive information to an attacker
Problem Description
Load below html Click Select a file Check in console u can see file name there Attaching video proof
<html>
<body onclick=“f()">click me</body>
<p></p>
<script>
function f() {
const filePromise = window.showOpenFilePicker();
setTimeout(async () => {
window.open(“https://google.com”)
const handle = (await filePromise)[0];
console.log(File: ${handle.name});
// …
}, 0);
}
</script>
</html>
Similar issue https://issues.chromium.org/40057597
Summary
Cross origin dialog spoof
Additional Data
Category: Security
Chrome Channel: Not sure
Regression: N/A