github/electron
1
1
Fork 0
mirror of https://github.com/electron/electron.git synced 2026-04-10 03:01:51 -04:00
Code Issues Packages Projects Releases Wiki Activity

chore: cherry-pick f1dd785e021e from chromium (#34561)

Browse Source 
* chore: cherry-pick f1dd785e021e from chromium

* fix patch

Co-authored-by: Electron Bot <electron@github.com>
This commit is contained in:
Jeremy Rose
2022-06-15 09:53:01 -07:00
committed by GitHub
parent 9e5aec2f75
commit 19c126ff6e
2 changed files with 230 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
patches/chromium/.patches
View File

@@ -140,4 +140,5 @@ cherry-pick-ec0cce63f47d.patch
cherry-pick-99c3f3bfd507.patch
cherry-pick-f1504440487f.patch
cherry-pick-21139756239b.patch
cherry-pick-f1dd785e021e.patch
cherry-pick-f3d01ff794dc.patch

229
patches/chromium/cherry-pick-f1dd785e021e.patch Normal file
View File

@@ -0,0 +1,229 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Austin Sullivan <asully@chromium.org>
Date: Mon, 16 May 2022 18:20:27 +0000
Subject: M102: FSA: Sanitize .scf files
.scf files can be used to execute code without opening the file.
Sanitize these files the same way we sanitize .lnk files.
Also updates filename sanitization logic to be consistent in blocking
.lnk and .local extensions on all OSes.
(cherry picked from commit 988164c6c4a563c3d4c0dedba295d09472dfc15f)
Bug: 1227995
Change-Id: I4b018f1ba524c783547e18630db9addc9fb126e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3089422
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1002147}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3648322
Auto-Submit: Austin Sullivan <asully@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/branch-heads/5005@{#759}
Cr-Branched-From: 5b4d9450fee01f821b6400e947b3839727643a71-refs/heads/main@{#992738}
diff --git a/content/browser/file_system_access/file_system_access_directory_handle_impl.cc b/content/browser/file_system_access/file_system_access_directory_handle_impl.cc
index 22e0fa6d237bda2819916d630a4de5583e56fe10..b6e3c97f044e1babbf8489b95e6db311ae8385b6 100644
--- a/content/browser/file_system_access/file_system_access_directory_handle_impl.cc
+++ b/content/browser/file_system_access/file_system_access_directory_handle_impl.cc
@@ -438,10 +438,13 @@ namespace {
bool IsShellIntegratedExtension(const base::FilePath::StringType& extension) {
base::FilePath::StringType extension_lower = base::ToLowerASCII(extension);
- // .lnk files may be used to execute arbitrary code (see
- // https://nvd.nist.gov/vuln/detail/CVE-2010-2568).
- if (extension_lower == FILE_PATH_LITERAL("lnk"))
+ // .lnk and .scf files may be used to execute arbitrary code (see
+ // https://nvd.nist.gov/vuln/detail/CVE-2010-2568 and
+ // https://crbug.com/1227995, respectively).
+ if (extension_lower == FILE_PATH_LITERAL("lnk") ||
+ extension_lower == FILE_PATH_LITERAL("scf")) {
return true;
+ }
// Setting a file's extension to a CLSID may conceal its actual file type on
// some Windows versions (see https://nvd.nist.gov/vuln/detail/CVE-2004-0420).
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 ac3fc8ceade37805dafcafd0b9e44eb2a196513a..e2b13687d664b0ee29c320d89cc6ee00d6134717 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
@@ -517,6 +517,16 @@ void FileSystemAccessManagerImpl::SetDefaultPathAndShowPicker(
suggested_name_path =
net::GenerateFileName(GURL(), std::string(), std::string(),
suggested_name, std::string(), std::string());
+
+ auto suggested_extension = suggested_name_path.Extension();
+ // Our version of `IsShellIntegratedExtension()` is more stringent than
+ // the version used in `net::GenerateFileName()`. See
+ // `FileSystemChooser::IsShellIntegratedExtension()` for details.
+ if (FileSystemChooser::IsShellIntegratedExtension(suggested_extension)) {
+ suggested_extension = FILE_PATH_LITERAL("download");
+ suggested_name_path =
+ suggested_name_path.ReplaceExtension(suggested_extension);
+ }
}
FileSystemChooser::Options file_system_chooser_options(
diff --git a/content/browser/file_system_access/file_system_chooser.cc b/content/browser/file_system_access/file_system_chooser.cc
index 6c5665aff03c28b8d4699aafcfda48648db3f47d..2f9d3b4dc999bbc7ddb16c05f2f1dd3ec12f6e15 100644
--- a/content/browser/file_system_access/file_system_chooser.cc
+++ b/content/browser/file_system_access/file_system_chooser.cc
@@ -71,33 +71,6 @@ base::FilePath::StringType GetLastExtension(
: extension;
}
-// Returns whether the specified extension receives special handling by the
-// Windows shell.
-bool IsShellIntegratedExtension(const base::FilePath::StringType& extension) {
- // TODO(https://crbug.com/1154757): Figure out some way to unify this with
- // net::IsSafePortablePathComponent, with the result probably ending up in
- // base/i18n/file_util_icu.h.
- base::FilePath::StringType extension_lower = base::ToLowerASCII(extension);
-
- // .lnk files may be used to execute arbitrary code (see
- // https://nvd.nist.gov/vuln/detail/CVE-2010-2568). .local files are used by
- // Windows to determine which DLLs to load for an application.
- if ((extension_lower == FILE_PATH_LITERAL("local")) ||
- (extension_lower == FILE_PATH_LITERAL("lnk"))) {
- return true;
- }
-
- // Setting a file's extension to a CLSID may conceal its actual file type on
- // some Windows versions (see https://nvd.nist.gov/vuln/detail/CVE-2004-0420).
- if (!extension_lower.empty() &&
- (extension_lower.front() == FILE_PATH_LITERAL('{')) &&
- (extension_lower.back() == FILE_PATH_LITERAL('}'))) {
- return true;
- }
-
- return false;
-}
-
// Extension validation primarily takes place in the renderer. This checks for a
// subset of invalid extensions in the event the renderer is compromised.
bool IsInvalidExtension(base::FilePath::StringType& extension) {
@@ -105,7 +78,7 @@ bool IsInvalidExtension(base::FilePath::StringType& extension) {
auto extension16 = base::UTF8ToUTF16(component8);
return !base::i18n::IsFilenameLegal(extension16) ||
- IsShellIntegratedExtension(GetLastExtension(extension));
+ FileSystemChooser::IsShellIntegratedExtension(extension);
}
// Converts the accepted mime types and extensions from `option` into a list
@@ -290,6 +263,40 @@ void FileSystemChooser::CreateAndShow(
/*params=*/nullptr);
}
+// static
+bool FileSystemChooser::IsShellIntegratedExtension(
+ const base::FilePath::StringType& extension) {
+ // TODO(https://crbug.com/1154757): Figure out some way to unify this with
+ // net::IsSafePortablePathComponent, with the result probably ending up in
+ // base/i18n/file_util_icu.h.
+ // - For the sake of consistency across platforms, we sanitize '.lnk' and
+ // '.local' files on all platforms (not just Windows)
+ // - There are some extensions (i.e. '.scf') we would like to sanitize which
+ // `net::GenerateFileName()` does not
+ base::FilePath::StringType extension_lower =
+ base::ToLowerASCII(GetLastExtension(extension));
+
+ // .lnk and .scf files may be used to execute arbitrary code (see
+ // https://nvd.nist.gov/vuln/detail/CVE-2010-2568 and
+ // https://crbug.com/1227995, respectively). .local files are used by Windows
+ // to determine which DLLs to load for an application.
+ if ((extension_lower == FILE_PATH_LITERAL("lnk")) ||
+ (extension_lower == FILE_PATH_LITERAL("local")) ||
+ (extension_lower == FILE_PATH_LITERAL("scf"))) {
+ return true;
+ }
+
+ // Setting a file's extension to a CLSID may conceal its actual file type on
+ // some Windows versions (see https://nvd.nist.gov/vuln/detail/CVE-2004-0420).
+ if (!extension_lower.empty() &&
+ (extension_lower.front() == FILE_PATH_LITERAL('{')) &&
+ (extension_lower.back() == FILE_PATH_LITERAL('}'))) {
+ return true;
+ }
+
+ return false;
+}
+
FileSystemChooser::FileSystemChooser(ui::SelectFileDialog::Type type,
ResultCallback callback,
base::ScopedClosureRunner fullscreen_block)
diff --git a/content/browser/file_system_access/file_system_chooser.h b/content/browser/file_system_access/file_system_chooser.h
index 07c8e7fda7d96496f58ed9a9e6cba6558c8c37df..925df7f5ef1d1bb94926afbe29beda248ae5aabc 100644
--- a/content/browser/file_system_access/file_system_chooser.h
+++ b/content/browser/file_system_access/file_system_chooser.h
@@ -68,6 +68,12 @@ class CONTENT_EXPORT FileSystemChooser : public ui::SelectFileDialog::Listener {
ResultCallback callback,
base::ScopedClosureRunner fullscreen_block);
+ // Returns whether the specified extension receives special handling by the
+ // Windows shell. These extensions should be sanitized before being shown in
+ // the "save as" file picker.
+ static bool IsShellIntegratedExtension(
+ const base::FilePath::StringType& extension);
+
FileSystemChooser(ui::SelectFileDialog::Type type,
ResultCallback callback,
base::ScopedClosureRunner fullscreen_block);
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 b5bebd08526f684337194a8cc0f08a0b345a65a7..6033251755d68d83bc5c8300630eaebacbd2f3ce 100644
--- a/content/browser/file_system_access/file_system_chooser_browsertest.cc
+++ b/content/browser/file_system_access/file_system_chooser_browsertest.cc
@@ -1557,21 +1557,28 @@ IN_PROC_BROWSER_TEST_F(FileSystemChooserBrowserTest, SuggestedName) {
"not_matching.jpg", false});
#if defined(OS_WIN)
- // ".local" and ".lnk" extensions should be sanitized on Windows.
+ // ".lnk", ".local", and ".scf" extensions should be sanitized.
name_infos.push_back({"dangerous_extension.local", ListValueOf(".local"),
true, "dangerous_extension.download", false});
name_infos.push_back({"dangerous_extension.lnk", ListValueOf(".lnk"), true,
"dangerous_extension.download", false});
-#else
- // ".local" and ".lnk" extensions should be allowed on other OSes.
- // TODO(https://crbug.com/1154757): `expected_exclude_accept_all_option` is
- // false here because ".local" and ".lnk" extensions are not allowed in
- // `accepts`, but are only sanitized by net::GenerateSafeFileName on Windows.
- name_infos.push_back({"dangerous_extension.local", ListValueOf(".local"),
- true, "dangerous_extension.local", false});
- name_infos.push_back({"dangerous_extension.lnk", ListValueOf(".lnk"), true,
- "dangerous_extension.lnk", false});
-#endif
+ name_infos.push_back({"dangerous_extension.scf", ListValueOf(".scf"), true,
+ "dangerous_extension.download", false});
+ // Compound extensions ending in a dangerous extension should be sanitized.
+ name_infos.push_back({"dangerous_extension.png.local", ListValueOf(".local"),
+ true, "dangerous_extension.png.download", false});
+ name_infos.push_back({"dangerous_extension.png.lnk", ListValueOf(".lnk"),
+ true, "dangerous_extension.png.download", false});
+ name_infos.push_back({"dangerous_extension.png.scf", ListValueOf(".scf"),
+ true, "dangerous_extension.png.download", false});
+ // Compound extensions not ending in a dangerous extension should not be
+ // sanitized.
+ name_infos.push_back({"dangerous_extension.local.png", ListValueOf(".png"),
+ true, "dangerous_extension.local.png", true});
+ name_infos.push_back({"dangerous_extension.lnk.png", ListValueOf(".png"),
+ true, "dangerous_extension.lnk.png", true});
+ name_infos.push_back({"dangerous_extension.scf.png", ListValueOf(".png"),
+ true, "dangerous_extension.scf.png", true});
// Invalid characters should be sanitized.
name_infos.push_back({R"(inv*l:d\\ch%rבאמת!a<ters🤓.txt)",
ListValueOf(".txt"), true,
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 373de41cf5ddead27b5e036c1cc14448a731250a..9b27d6305bd00a19d94b5ec49f21a7ecff7ddc48 100644
--- a/content/browser/file_system_access/file_system_chooser_unittest.cc
+++ b/content/browser/file_system_access/file_system_chooser_unittest.cc
@@ -189,7 +189,7 @@ TEST_F(FileSystemChooserTest, IgnoreShellIntegratedExtensions) {
accepts.emplace_back(blink::mojom::ChooseFileSystemEntryAcceptsOption::New(
u"", std::vector<std::string>({}),
std::vector<std::string>(
- {"lnk", "foo.lnk", "foo.bar.local", "text", "local"})));
+ {"lnk", "foo.lnk", "foo.bar.local", "text", "local", "scf"})));
SyncShowDialog(std::move(accepts), /*include_accepts_all=*/false);
ASSERT_TRUE(dialog_params.file_types);