chore: cherry-pick a66dbdcf6493 from chromium (#28294)

* chore: cherry-pick a66dbdcf6493 from chromium

* update patches

Co-authored-by: Electron Bot <[email protected]>

patches/chromium/.patches

@@ -139,3 +139,4 @@ cherry-pick-33861dcf6d15.patch
 cherry-pick-93ce5606cd9a.patch
 cherry-pick-c6d6f7aee733.patch
 cherry-pick-e1505713dc31.patch
+cherry-pick-a66dbdcf6493.patch

patches/chromium/cherry-pick-a66dbdcf6493.patch

@@ -0,0 +1,475 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Marijn Kruisselbrink <[email protected]>
+Date: Tue, 8 Dec 2020 07:20:47 +0000
+Subject: Reland "Reland "[FSA] Add IsSafePathComponent checks to
+ GetFile/GetDirectoryHandle.""
+
+This is a reland of 2d41c3952d2851948a09ddcf3e97bae6c419b024
+
+The added test was modified to no longer assert that all unsafe files
+were written to disk successfully. This should make the test pass (albeit
+with less stringent checks) on file systems/platforms that don't allow
+all unsafe file names.
+
+Original change's description:
+> Reland "[FSA] Add IsSafePathComponent checks to GetFile/GetDirectoryHandle."
+>
+> This is a reland of 004377929febd7cf7392932b01df7f4a0a362679
+>
+> The main difference is to make sure iterating over a directory doesn't
+> return files we don't want to expose either (and not CHECK failing if
+> such files are found when iterating).
+>
+> Original change's description:
+> > [FSA] Add IsSafePathComponent checks to GetFile/GetDirectoryHandle.
+> >
+> > This isn't directly using net::IsSafePortablePathComponent since what
+> > is safe for the File System Access API is not the same as what is safe
+> > for Downloads. As such currently this duplicates a lot of the
+> > implementation of this method, but in a followup we should attempt to
+> > unify these two implementations as much as possible.
+> >
+> > Bug: 1150810, 1154757
+> > Change-Id: Iba4c92ef5f1cd924aa22b9dd201762d48b4bbc3b
+> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2568383
+> > Commit-Queue: Marijn Kruisselbrink <[email protected]>
+> > Reviewed-by: Victor Costan <[email protected]>
+> > Cr-Commit-Position: refs/heads/master@{#833042}
+>
+> Bug: 1150810
+> Bug: 1154757
+> Change-Id: I3341b9824a1ac4cbd6f100355960ad55b01f0753
+> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2575370
+> Commit-Queue: Victor Costan <[email protected]>
+> Reviewed-by: Victor Costan <[email protected]>
+> Cr-Commit-Position: refs/heads/master@{#834118}
+
+Bug: 1150810
+Bug: 1154757
+Change-Id: Ie5cad9a7b2383c89b96e8a7be6cfe75ad2555fa6
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2577614
+Commit-Queue: Marijn Kruisselbrink <[email protected]>
+Auto-Submit: Marijn Kruisselbrink <[email protected]>
+Reviewed-by: Victor Costan <[email protected]>
+Cr-Commit-Position: refs/heads/master@{#834598}
+
+diff --git a/content/browser/file_system_access/native_file_system_directory_handle_impl.cc b/content/browser/file_system_access/native_file_system_directory_handle_impl.cc
+index de58fb9b60e78e68ea807de0f800266f71408cc2..2472c899621dab33c61a30d24058cd5dffe83e8b 100644
+--- a/content/browser/file_system_access/native_file_system_directory_handle_impl.cc
++++ b/content/browser/file_system_access/native_file_system_directory_handle_impl.cc
+@@ -4,6 +4,7 @@
+ 
+ #include "content/browser/file_system_access/native_file_system_directory_handle_impl.h"
+ 
++#include "base/i18n/file_util_icu.h"
+ #include "base/strings/strcat.h"
+ #include "base/strings/string_util.h"
+ #include "base/strings/utf_string_conversions.h"
+@@ -12,6 +13,7 @@
+ #include "content/browser/file_system_access/native_file_system_transfer_token_impl.h"
+ #include "mojo/public/cpp/bindings/pending_remote.h"
+ #include "net/base/escape.h"
++#include "net/base/filename_util.h"
+ #include "storage/browser/file_system/file_system_context.h"
+ #include "storage/browser/file_system/file_system_operation_runner.h"
+ #include "storage/common/file_system/file_system_util.h"
+@@ -30,27 +32,6 @@ namespace content {
+ 
+ using HandleType = NativeFileSystemPermissionContext::HandleType;
+ 
+-namespace {
+-
+-// Returns true when |name| contains a path separator like "/".
+-bool ContainsPathSeparator(const std::string& name) {
+-  const base::FilePath filepath_name = storage::StringToFilePath(name);
+-
+-  const size_t separator_position =
+-      filepath_name.value().find_first_of(base::FilePath::kSeparators);
+-
+-  return separator_position != base::FilePath::StringType::npos;
+-}
+-
+-// Returns true when |name| is "." or "..".
+-bool IsCurrentOrParentDirectory(const std::string& name) {
+-  const base::FilePath filepath_name = storage::StringToFilePath(name);
+-  return filepath_name.value() == base::FilePath::kCurrentDirectory ||
+-         filepath_name.value() == base::FilePath::kParentDirectory;
+-}
+-
+-}  // namespace
+-
+ NativeFileSystemDirectoryHandleImpl::NativeFileSystemDirectoryHandleImpl(
+     NativeFileSystemManagerImpl* manager,
+     const BindingContext& context,
+@@ -379,9 +360,10 @@ void NativeFileSystemDirectoryHandleImpl::DidReadDirectory(
+     blink::mojom::NativeFileSystemErrorPtr get_child_url_result =
+         GetChildURL(basename, &child_url);
+ 
+-    // All entries must exist in this directory as a direct child with a valid
+-    // |basename|.
+-    CHECK_EQ(get_child_url_result->status, NativeFileSystemStatus::kOk);
++    // Skip any entries with names that aren't allowed to be accessed by
++    // this API, such as files with disallowed characters in their names.
++    if (get_child_url_result->status != NativeFileSystemStatus::kOk)
++      continue;
+ 
+     entries.push_back(
+         CreateEntry(basename, child_url,
+@@ -412,25 +394,97 @@ void NativeFileSystemDirectoryHandleImpl::RemoveEntryImpl(
+       url, recurse);
+ }
+ 
++namespace {
++
++// Returns whether the specified extension receives special handling by the
++// 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"))
++    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;
++}
++
++}  // namespace
++
++// static
++bool NativeFileSystemDirectoryHandleImpl::IsSafePathComponent(
++    const std::string& name) {
++  // This method is similar to net::IsSafePortablePathComponent, with a few
++  // notable differences where the net version does not consider names safe
++  // while here we do want to allow them. These cases are:
++  //  - Names starting with a '.'. These would be hidden files in most file
++  //    managers, but are something we explicitly want to support for the
++  //    File System Access API, for names like .git.
++  //  - Names that end in '.local'. For downloads writing to such files is
++  //    dangerous since it might modify what code is executed when an executable
++  //    is ran from the same directory. For the File System Access API this
++  //    isn't really a problem though, since if a website can write to a .local
++  //    file via a FileSystemDirectoryHandle they can also just modify the
++  //    executables in the directory directly.
++  //
++  // TODO(https://crbug.com/1154757): Unify this with
++  // net::IsSafePortablePathComponent, with the result probably ending up in
++  // base/i18n/file_util_icu.h.
++
++  const base::FilePath component = storage::StringToFilePath(name);
++  // Empty names, or names that contain path separators are invalid.
++  if (component.empty() || component != component.BaseName() ||
++      component != component.StripTrailingSeparators()) {
++    return false;
++  }
++
++  base::string16 component16;
++#if defined(OS_WIN)
++  component16.assign(component.value().begin(), component.value().end());
++#else
++  std::string component8 = component.AsUTF8Unsafe();
++  if (!base::UTF8ToUTF16(component8.c_str(), component8.size(), &component16))
++    return false;
++#endif
++  // base::i18n::IsFilenameLegal blocks names that start with '.', so strip out
++  // a leading '.' before passing it to that method.
++  // TODO(mek): Consider making IsFilenameLegal more flexible to support this
++  // use case.
++  if (component16[0] == '.')
++    component16 = component16.substr(1);
++  if (!base::i18n::IsFilenameLegal(component16))
++    return false;
++
++  base::FilePath::StringType extension = component.Extension();
++  if (!extension.empty())
++    extension.erase(extension.begin());  // Erase preceding '.'.
++  if (IsShellIntegratedExtension(extension))
++    return false;
++
++  if (base::TrimString(component.value(), FILE_PATH_LITERAL("."),
++                       base::TRIM_TRAILING) != component.value()) {
++    return false;
++  }
++
++  if (net::IsReservedNameOnWindows(component.value()))
++    return false;
++
++  return true;
++}
++
+ blink::mojom::NativeFileSystemErrorPtr
+ NativeFileSystemDirectoryHandleImpl::GetChildURL(
+     const std::string& basename,
+     storage::FileSystemURL* result) {
+-  // TODO(mek): Rather than doing URL serialization and parsing we should just
+-  // have a way to get a child FileSystemURL directly from its parent.
+-
+-  if (basename.empty()) {
+-    return native_file_system_error::FromStatus(
+-        NativeFileSystemStatus::kInvalidArgument,
+-        "Name can't be an empty string.");
+-  }
+-
+-  if (ContainsPathSeparator(basename) || IsCurrentOrParentDirectory(basename)) {
+-    // |basename| must refer to a entry that exists in this directory as a
+-    // direct child.
++  if (!IsSafePathComponent(basename)) {
+     return native_file_system_error::FromStatus(
+-        NativeFileSystemStatus::kInvalidArgument,
+-        "Name contains invalid characters.");
++        NativeFileSystemStatus::kInvalidArgument, "Name is not allowed.");
+   }
+ 
+   const storage::FileSystemURL parent = url();
+diff --git a/content/browser/file_system_access/native_file_system_directory_handle_impl.h b/content/browser/file_system_access/native_file_system_directory_handle_impl.h
+index 360fdbc5851f804121ec6d4fbb069c1cd9429fd2..537b2a68511523c01853a95b304765c88d52ed3a 100644
+--- a/content/browser/file_system_access/native_file_system_directory_handle_impl.h
++++ b/content/browser/file_system_access/native_file_system_directory_handle_impl.h
+@@ -57,6 +57,14 @@ class CONTENT_EXPORT NativeFileSystemDirectoryHandleImpl
+       mojo::PendingReceiver<blink::mojom::NativeFileSystemTransferToken> token)
+       override;
+ 
++  // The File System Access API should not give access to files that might
++  // trigger special handling from the operating system. This method is used to
++  // validate that all paths passed to GetFileHandle/GetDirectoryHandle are safe
++  // to be exposed to the web.
++  // TODO(https://crbug.com/1154757): Merge this with
++  // net::IsSafePortablePathComponent.
++  static bool IsSafePathComponent(const std::string& name);
++
+  private:
+   // This method creates the file if it does not currently exists. I.e. it is
+   // the implementation for passing create=true to GetFile.
+diff --git a/content/browser/file_system_access/native_file_system_directory_handle_impl_unittest.cc b/content/browser/file_system_access/native_file_system_directory_handle_impl_unittest.cc
+new file mode 100644
+index 0000000000000000000000000000000000000000..2601bc93d2561292b3972c53699d65d40356dc39
+--- /dev/null
++++ b/content/browser/file_system_access/native_file_system_directory_handle_impl_unittest.cc
+@@ -0,0 +1,196 @@
++// Copyright 2020 The Chromium Authors. All rights reserved.
++// Use of this source code is governed by a BSD-style license that can be
++// found in the LICENSE file.
++
++#include "content/browser/file_system_access/native_file_system_directory_handle_impl.h"
++
++#include <iterator>
++#include <string>
++
++#include "base/bind.h"
++#include "base/files/file_util.h"
++#include "base/files/scoped_temp_dir.h"
++#include "base/macros.h"
++#include "base/run_loop.h"
++#include "base/test/bind.h"
++#include "base/test/task_environment.h"
++#include "build/build_config.h"
++#include "content/browser/file_system_access/fixed_native_file_system_permission_grant.h"
++#include "content/public/test/browser_task_environment.h"
++#include "storage/browser/test/test_file_system_context.h"
++#include "testing/gmock/include/gmock/gmock.h"
++#include "testing/gtest/include/gtest/gtest.h"
++
++namespace content {
++
++using storage::FileSystemURL;
++
++class NativeFileSystemDirectoryHandleImplTest : public testing::Test {
++ public:
++  NativeFileSystemDirectoryHandleImplTest()
++      : task_environment_(base::test::TaskEnvironment::MainThreadType::IO) {}
++
++  void SetUp() override {
++    ASSERT_TRUE(dir_.CreateUniqueTempDir());
++
++    file_system_context_ = storage::CreateFileSystemContextForTesting(
++        /*quota_manager_proxy=*/nullptr, dir_.GetPath());
++
++    chrome_blob_context_ = base::MakeRefCounted<ChromeBlobStorageContext>();
++    chrome_blob_context_->InitializeOnIOThread(base::FilePath(),
++                                               base::FilePath(), nullptr);
++
++    manager_ = base::MakeRefCounted<NativeFileSystemManagerImpl>(
++        file_system_context_, chrome_blob_context_,
++        /*permission_context=*/nullptr,
++        /*off_the_record=*/false);
++
++    auto url_and_fs = manager_->CreateFileSystemURLFromPath(
++        test_src_origin_, NativeFileSystemEntryFactory::PathType::kLocal,
++        dir_.GetPath());
++
++    handle_ = std::make_unique<NativeFileSystemDirectoryHandleImpl>(
++        manager_.get(),
++        NativeFileSystemManagerImpl::BindingContext(
++            test_src_origin_, test_src_url_, /*worker_process_id=*/1),
++        url_and_fs.url,
++        NativeFileSystemManagerImpl::SharedHandleState(
++            allow_grant_, allow_grant_, std::move(url_and_fs.file_system)));
++  }
++
++  void TearDown() override { task_environment_.RunUntilIdle(); }
++
++ protected:
++  const GURL test_src_url_ = GURL("http://example.com/foo");
++  const url::Origin test_src_origin_ = url::Origin::Create(test_src_url_);
++
++  BrowserTaskEnvironment task_environment_;
++
++  base::ScopedTempDir dir_;
++  scoped_refptr<storage::FileSystemContext> file_system_context_;
++  scoped_refptr<ChromeBlobStorageContext> chrome_blob_context_;
++  scoped_refptr<NativeFileSystemManagerImpl> manager_;
++
++  scoped_refptr<FixedNativeFileSystemPermissionGrant> allow_grant_ =
++      base::MakeRefCounted<FixedNativeFileSystemPermissionGrant>(
++          FixedNativeFileSystemPermissionGrant::PermissionStatus::GRANTED,
++          base::FilePath());
++  std::unique_ptr<NativeFileSystemDirectoryHandleImpl> handle_;
++};
++
++TEST_F(NativeFileSystemDirectoryHandleImplTest, IsSafePathComponent) {
++  constexpr const char* kSafePathComponents[] = {
++      "a", "a.txt", "a b.txt", "My Computer", ".a", "lnk.zip", "lnk", "a.local",
++  };
++
++  constexpr const char* kUnsafePathComponents[] = {
++      "",
++      ".",
++      "..",
++      "...",
++      "con",
++      "con.zip",
++      "NUL",
++      "NUL.zip",
++      "a.",
++      "a\"a",
++      "a<a",
++      "a>a",
++      "a?a",
++      "a/",
++      "a\\",
++      "a ",
++      "a . .",
++      " Computer",
++      "My Computer.{a}",
++      "My Computer.{20D04FE0-3AEA-1069-A2D8-08002B30309D}",
++      "a\\a",
++      "a.lnk",
++      "a/a",
++      "C:\\",
++      "C:/",
++      "C:",
++  };
++
++  for (const char* component : kSafePathComponents) {
++    EXPECT_TRUE(
++        NativeFileSystemDirectoryHandleImpl::IsSafePathComponent(component))
++        << component;
++  }
++  for (const char* component : kUnsafePathComponents) {
++    EXPECT_FALSE(
++        NativeFileSystemDirectoryHandleImpl::IsSafePathComponent(component))
++        << component;
++  }
++}
++
++namespace {
++class TestNativeFileSystemDirectoryEntriesListener
++    : public blink::mojom::NativeFileSystemDirectoryEntriesListener {
++ public:
++  TestNativeFileSystemDirectoryEntriesListener(
++      std::vector<blink::mojom::NativeFileSystemEntryPtr>* entries,
++      base::OnceClosure done)
++      : entries_(entries), done_(std::move(done)) {}
++
++  void DidReadDirectory(
++      blink::mojom::NativeFileSystemErrorPtr result,
++      std::vector<blink::mojom::NativeFileSystemEntryPtr> entries,
++      bool has_more_entries) override {
++    EXPECT_EQ(result->status, blink::mojom::NativeFileSystemStatus::kOk);
++    entries_->insert(entries_->end(), std::make_move_iterator(entries.begin()),
++                     std::make_move_iterator(entries.end()));
++    if (!has_more_entries) {
++      std::move(done_).Run();
++    }
++  }
++
++ private:
++  std::vector<blink::mojom::NativeFileSystemEntryPtr>* entries_;
++  base::OnceClosure done_;
++};
++}  // namespace
++
++TEST_F(NativeFileSystemDirectoryHandleImplTest, GetEntries) {
++  constexpr const char* kSafeNames[] = {"a", "a.txt", "My Computer", "lnk.txt",
++                                        "a.local"};
++  constexpr const char* kUnsafeNames[] = {
++      "con",  "con.zip", "NUL",   "a.",
++      "a\"a", "a . .",   "a.lnk", "My Computer.{a}",
++  };
++  for (const char* name : kSafeNames) {
++    ASSERT_TRUE(base::WriteFile(dir_.GetPath().AppendASCII(name), "data"))
++        << name;
++  }
++  for (const char* name : kUnsafeNames) {
++    base::FilePath file_path = dir_.GetPath().AppendASCII(name);
++    bool success = base::WriteFile(file_path, "data");
++#if !defined(OS_WIN)
++    // Some of the unsafe names are not legal file names on Windows. This is
++    // okay, and doesn't materially effect the outcome of the test, so just
++    // ignore any failures writing these files to disk.
++    EXPECT_TRUE(success) << "Failed to create file " << file_path;
++#else
++    ignore_result(success);
++#endif
++  }
++
++  std::vector<blink::mojom::NativeFileSystemEntryPtr> entries;
++  base::RunLoop loop;
++  mojo::PendingRemote<blink::mojom::NativeFileSystemDirectoryEntriesListener>
++      listener;
++  mojo::MakeSelfOwnedReceiver(
++      std::make_unique<TestNativeFileSystemDirectoryEntriesListener>(
++          &entries, loop.QuitClosure()),
++      listener.InitWithNewPipeAndPassReceiver());
++  handle_->GetEntries(std::move(listener));
++  loop.Run();
++
++  std::vector<std::string> names;
++  for (const auto& entry : entries) {
++    names.push_back(entry->name);
++  }
++  EXPECT_THAT(names, testing::UnorderedElementsAreArray(kSafeNames));
++}
++
++}  // namespace content
+diff --git a/content/browser/file_system_access/native_file_system_file_handle_impl_unittest.cc b/content/browser/file_system_access/native_file_system_file_handle_impl_unittest.cc
+index c8f3ec61d9d90aecb978506bac364c386574c780..444cbe9bde90280993388d96782abaeb87c0c6b6 100644
+--- a/content/browser/file_system_access/native_file_system_file_handle_impl_unittest.cc
++++ b/content/browser/file_system_access/native_file_system_file_handle_impl_unittest.cc
+@@ -28,8 +28,6 @@
+ #include "storage/browser/test/test_file_system_context.h"
+ #include "testing/gtest/include/gtest/gtest.h"
+ 
+-using storage::FileSystemURL;
+-
+ namespace content {
+ 
+ using blink::mojom::PermissionStatus;
+diff --git a/content/test/BUILD.gn b/content/test/BUILD.gn
+index 003f7fe21194b1aeae1db33238016749f3d2b835..4814b6599240d21166e7c9432643c02b3104d2ef 100644
+--- a/content/test/BUILD.gn
++++ b/content/test/BUILD.gn
+@@ -1745,6 +1745,7 @@ test("content_unittests") {
+     "../browser/file_system/browser_file_system_helper_unittest.cc",
+     "../browser/file_system/file_system_operation_runner_unittest.cc",
+     "../browser/file_system_access/file_system_chooser_unittest.cc",
++    "../browser/file_system_access/native_file_system_directory_handle_impl_unittest.cc",
+     "../browser/file_system_access/native_file_system_file_handle_impl_unittest.cc",
+     "../browser/file_system_access/native_file_system_file_writer_impl_unittest.cc",
+     "../browser/file_system_access/native_file_system_handle_base_unittest.cc",