summaryrefslogtreecommitdiffstats
path: root/chromium
diff options
context:
space:
mode:
authorAustin Sullivan <asully@chromium.org>2022-05-16 18:20:27 +0000
committerMichael BrĂ¼ning <michael.bruning@qt.io>2022-07-25 16:36:14 +0000
commit01ca651248da6fd6d6666d5b959d81858b25510f (patch)
tree4338b061740fb317095d762a88c07f74c834f7f6 /chromium
parent945b84abb15534898fcac312112b86831c2fc081 (diff)
[Backport] CVE-2022-1857: Insufficient policy enforcement in File System API
Manual backport of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/3648322: 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-by: Marijn Kruisselbrink <mek@chromium.org> Commit-Queue: Marijn Kruisselbrink <mek@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1002147} 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} Reviewed-by: Michal Klocek <michal.klocek@qt.io>
Diffstat (limited to 'chromium')
-rw-r--r--chromium/content/browser/file_system_access/file_system_chooser.cc60
-rw-r--r--chromium/content/browser/file_system_access/file_system_chooser.h6
-rw-r--r--chromium/content/browser/file_system_access/native_file_system_directory_handle_impl.cc9
3 files changed, 47 insertions, 28 deletions
diff --git a/chromium/content/browser/file_system_access/file_system_chooser.cc b/chromium/content/browser/file_system_access/file_system_chooser.cc
index 32b1934f342..c72148abcba 100644
--- a/chromium/content/browser/file_system_access/file_system_chooser.cc
+++ b/chromium/content/browser/file_system_access/file_system_chooser.cc
@@ -69,30 +69,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) {
@@ -100,7 +76,7 @@ bool IsInvalidExtension(base::FilePath::StringType& extension) {
auto extension16 = base::UTF8ToUTF16(component8.c_str());
return !base::i18n::IsFilenameLegal(extension16) ||
- IsShellIntegratedExtension(GetLastExtension(extension));
+ FileSystemChooser::IsShellIntegratedExtension(GetLastExtension(extension));
}
// Converts the accepted mime types and extensions from |option| into a list
@@ -256,6 +232,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(
blink::mojom::ChooseFileSystemEntryType type,
ResultCallback callback,
diff --git a/chromium/content/browser/file_system_access/file_system_chooser.h b/chromium/content/browser/file_system_access/file_system_chooser.h
index 3508832ce49..933744a3695 100644
--- a/chromium/content/browser/file_system_access/file_system_chooser.h
+++ b/chromium/content/browser/file_system_access/file_system_chooser.h
@@ -62,6 +62,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(blink::mojom::ChooseFileSystemEntryType type,
ResultCallback callback,
base::ScopedClosureRunner fullscreen_block);
diff --git a/chromium/content/browser/file_system_access/native_file_system_directory_handle_impl.cc b/chromium/content/browser/file_system_access/native_file_system_directory_handle_impl.cc
index 2992957115d..98714c00b9c 100644
--- a/chromium/content/browser/file_system_access/native_file_system_directory_handle_impl.cc
+++ b/chromium/content/browser/file_system_access/native_file_system_directory_handle_impl.cc
@@ -407,10 +407,13 @@ namespace {
// Windows shell.
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).
if (!extension_lower.empty() &&