Add a recursive traversal to verify that an unpacked extension is not…

… using a Windows reserved filename.

BUG=428511

Review URL: https://codereview.chromium.org/1219983002

Cr-Commit-Position: refs/heads/master@{#337742}

extensions/common/file_util.cc

@@ -37,6 +37,7 @@
 #include "extensions/common/manifest_handlers/shared_module_info.h"
 #include "grit/extensions_strings.h"
 #include "net/base/escape.h"
+#include "net/base/filename_util.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "url/gurl.h"
 
@@ -268,6 +269,13 @@ bool ValidateExtension(const Extension* extension,
   if (!CheckForIllegalFilenames(extension->path(), &warning))
     warnings->push_back(InstallWarning(warning));
 
+  // Check that the extension does not include any Windows reserved filenames.
+  std::string windows_reserved_warning;
+  if (!CheckForWindowsReservedFilenames(extension->path(),
+                                        &windows_reserved_warning)) {
+    warnings->push_back(InstallWarning(windows_reserved_warning));
+  }
+
   // Check that extensions don't include private key files.
   std::vector<base::FilePath> private_keys =
       FindPrivateKeyFiles(extension->path());
@@ -338,6 +346,7 @@ bool CheckForIllegalFilenames(const base::FilePath& extension_path,
   base::FilePath file;
   while (!(file = all_files.Next()).empty()) {
     base::FilePath::StringType filename = file.BaseName().value();
+
     // Skip all that don't start with "_".
     if (filename.find_first_of(FILE_PATH_LITERAL("_")) != 0)
       continue;
@@ -354,6 +363,28 @@ bool CheckForIllegalFilenames(const base::FilePath& extension_path,
   return true;
 }
 
+bool CheckForWindowsReservedFilenames(const base::FilePath& extension_dir,
+                                      std::string* error) {
+  const int kFilesAndDirectories =
+      base::FileEnumerator::DIRECTORIES | base::FileEnumerator::FILES;
+  base::FileEnumerator traversal(extension_dir, true, kFilesAndDirectories);
+
+  for (base::FilePath current = traversal.Next(); !current.empty();
+       current = traversal.Next()) {
+    base::FilePath::StringType filename = current.BaseName().value();
+    bool is_reserved_filename = net::IsReservedNameOnWindows(filename);
+    if (is_reserved_filename) {
+      *error = base::StringPrintf(
+          "Cannot load extension with file or directory name %s. "
+          "The filename is illegal.",
+          current.BaseName().AsUTF8Unsafe().c_str());
+      return false;
+    }
+  }
+
+  return true;
+}
+
 base::FilePath GetInstallTempDir(const base::FilePath& extensions_dir) {
   // We do file IO in this function, but only when the current profile's
   // Temp directory has never been used before, or in a rare error case.

extensions/common/file_util.h

@@ -89,6 +89,13 @@ std::vector<base::FilePath> FindPrivateKeyFiles(
 bool CheckForIllegalFilenames(const base::FilePath& extension_path,
                               std::string* error);
 
+// We need to reserve the names of special Windows filenames, such as
+// "com2.zip."
+// If any files or directories are found to be using a reserved Windows
+// filename, we return false, and set error message.
+bool CheckForWindowsReservedFilenames(const base::FilePath& extension_dir,
+                                      std::string* error);
+
 // Returns a path to a temporary directory for unpacking an extension that will
 // be installed into |extensions_dir|. Creates the directory if necessary.
 // The directory will be on the same file system as |extensions_dir| so

extensions/common/file_util_unittest.cc

@@ -187,6 +187,40 @@ TEST_F(FileUtilTest, CheckIllegalFilenamesReservedAndIllegal) {
   EXPECT_FALSE(file_util::CheckForIllegalFilenames(temp.path(), &error));
 }
 
+// These tests do not work on Windows, because it is illegal to create a
+// file/directory with a Windows reserved name. Because we cannot create a
+// file that will cause the test to fail, let's skip the test.
+#if !defined(OS_WIN)
+TEST_F(FileUtilTest, CheckIllegalFilenamesDirectoryWindowsReserved) {
+  base::ScopedTempDir temp;
+  ASSERT_TRUE(temp.CreateUniqueTempDir());
+
+  base::FilePath src_path = temp.path().AppendASCII("aux");
+  ASSERT_TRUE(base::CreateDirectory(src_path));
+
+  std::string error;
+  EXPECT_FALSE(
+      file_util::CheckForWindowsReservedFilenames(temp.path(), &error));
+}
+
+TEST_F(FileUtilTest,
+       CheckIllegalFilenamesWindowsReservedFilenameWithExtension) {
+  base::ScopedTempDir temp;
+  ASSERT_TRUE(temp.CreateUniqueTempDir());
+
+  base::FilePath src_path = temp.path().AppendASCII("some_dir");
+  ASSERT_TRUE(base::CreateDirectory(src_path));
+
+  std::string data = "{ \"name\": { \"message\": \"foobar\" } }";
+  ASSERT_TRUE(base::WriteFile(src_path.AppendASCII("lpt1.txt"), data.c_str(),
+                              data.length()));
+
+  std::string error;
+  EXPECT_FALSE(
+      file_util::CheckForWindowsReservedFilenames(temp.path(), &error));
+}
+#endif
+
 TEST_F(FileUtilTest, LoadExtensionGivesHelpfullErrorOnMissingManifest) {
   base::FilePath install_dir;
   ASSERT_TRUE(PathService::Get(DIR_TEST_DATA, &install_dir));

net/base/filename_util.cc

@@ -141,7 +141,7 @@ void GenerateSafeFileName(const std::string& mime_type,
   // Prepend "_" to the file name if it's a reserved name
   base::FilePath::StringType leaf_name = file_path->BaseName().value();
   DCHECK(!leaf_name.empty());
-  if (IsReservedName(leaf_name)) {
+  if (IsReservedNameOnWindows(leaf_name)) {
     leaf_name = base::FilePath::StringType(FILE_PATH_LITERAL("_")) + leaf_name;
     *file_path = file_path->DirName();
     if (file_path->value() == base::FilePath::kCurrentDirectory) {
@@ -153,4 +153,44 @@ void GenerateSafeFileName(const std::string& mime_type,
 #endif
 }
 
+bool IsReservedNameOnWindows(const base::FilePath::StringType& filename) {
+  // This list is taken from the MSDN article "Naming a file"
+  // http://msdn2.microsoft.com/en-us/library/aa365247(VS.85).aspx
+  // I also added clock$ because GetSaveFileName seems to consider it as a
+  // reserved name too.
+  static const char* const known_devices[] = {
+      "con",  "prn",  "aux",  "nul",  "com1", "com2", "com3",  "com4",
+      "com5", "com6", "com7", "com8", "com9", "lpt1", "lpt2",  "lpt3",
+      "lpt4", "lpt5", "lpt6", "lpt7", "lpt8", "lpt9", "clock$"};
+#if defined(OS_WIN)
+  std::string filename_lower =
+      base::StringToLowerASCII(base::WideToUTF8(filename));
+#elif defined(OS_POSIX)
+  std::string filename_lower = base::StringToLowerASCII(filename);
+#endif
+
+  for (size_t i = 0; i < arraysize(known_devices); ++i) {
+    // Exact match.
+    if (filename_lower == known_devices[i])
+      return true;
+    // Starts with "DEVICE.".
+    if (filename_lower.find(std::string(known_devices[i]) + ".") == 0)
+      return true;
+  }
+
+  static const char* const magic_names[] = {
+      // These file names are used by the "Customize folder" feature of the
+      // shell.
+      "desktop.ini",
+      "thumbs.db",
+  };
+
+  for (size_t i = 0; i < arraysize(magic_names); ++i) {
+    if (filename_lower == magic_names[i])
+      return true;
+  }
+
+  return false;
+}
+
 }  // namespace net

net/base/filename_util.h

@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/files/file_path.h"
 #include "base/strings/string16.h"
 #include "net/base/net_export.h"
 
@@ -111,6 +112,14 @@ NET_EXPORT void GenerateSafeFileName(const std::string& mime_type,
                                      bool ignore_extension,
                                      base::FilePath* file_path);
 
+// Returns whether the specified file name is a reserved name on Windows.
+// This includes names like "com2.zip" (which correspond to devices) and
+// desktop.ini and thumbs.db which have special meaning to the Windows shell.
+// Even on other platforms, this will return whether or not a file name is
+// reserved on Windows.
+NET_EXPORT bool IsReservedNameOnWindows(
+    const base::FilePath::StringType& filename);
+
 }  // namespace net
 
 #endif  // NET_BASE_FILENAME_UTIL_H_

net/base/filename_util_icu.cc

@@ -26,7 +26,8 @@ bool IsSafePortablePathComponent(const base::FilePath& component) {
          FilePathToString16(component, &component16) &&
          base::i18n::IsFilenameLegal(component16) &&
          !IsShellIntegratedExtension(extension) &&
-         (sanitized == component.value()) && !IsReservedName(component.value());
+         (sanitized == component.value()) &&
+         !IsReservedNameOnWindows(component.value());
 }
 
 bool IsSafePortableRelativePath(const base::FilePath& path) {

net/base/filename_util_internal.cc

@@ -108,48 +108,6 @@ bool IsShellIntegratedExtension(const base::FilePath::StringType& extension) {
   return false;
 }
 
-// Returns whether the specified file name is a reserved name on windows.
-// This includes names like "com2.zip" (which correspond to devices) and
-// desktop.ini and thumbs.db which have special meaning to the windows shell.
-bool IsReservedName(const base::FilePath::StringType& filename) {
-  // This list is taken from the MSDN article "Naming a file"
-  // http://msdn2.microsoft.com/en-us/library/aa365247(VS.85).aspx
-  // I also added clock$ because GetSaveFileName seems to consider it as a
-  // reserved name too.
-  static const char* const known_devices[] = {
-      "con",  "prn",  "aux",  "nul",  "com1", "com2", "com3",  "com4",
-      "com5", "com6", "com7", "com8", "com9", "lpt1", "lpt2",  "lpt3",
-      "lpt4", "lpt5", "lpt6", "lpt7", "lpt8", "lpt9", "clock$"};
-#if defined(OS_WIN)
-  std::string filename_lower =
-      base::StringToLowerASCII(base::WideToUTF8(filename));
-#elif defined(OS_POSIX)
-  std::string filename_lower = base::StringToLowerASCII(filename);
-#endif
-
-  for (size_t i = 0; i < arraysize(known_devices); ++i) {
-    // Exact match.
-    if (filename_lower == known_devices[i])
-      return true;
-    // Starts with "DEVICE.".
-    if (filename_lower.find(std::string(known_devices[i]) + ".") == 0)
-      return true;
-  }
-
-  static const char* const magic_names[] = {
-    // These file names are used by the "Customize folder" feature of the shell.
-    "desktop.ini",
-    "thumbs.db",
-  };
-
-  for (size_t i = 0; i < arraysize(magic_names); ++i) {
-    if (filename_lower == magic_names[i])
-      return true;
-  }
-
-  return false;
-}
-
 // Examines the current extension in |file_name| and modifies it if necessary in
 // order to ensure the filename is safe.  If |file_name| doesn't contain an
 // extension or if |ignore_extension| is true, then a new extension will be

net/base/filename_util_internal.h

@@ -26,8 +26,6 @@ void SanitizeGeneratedFileName(base::FilePath::StringType* filename,
 
 bool IsShellIntegratedExtension(const base::FilePath::StringType& extension);
 
-bool IsReservedName(const base::FilePath::StringType& filename);
-
 void EnsureSafeExtension(const std::string& mime_type,
                          bool ignore_extension,
                          base::FilePath* file_name);

net/base/filename_util_unittest.cc

@@ -115,6 +115,13 @@ static const base::FilePath::CharType* kUnsafePortableBasenames[] = {
 #endif
 };
 
+static const base::FilePath::CharType* kUnsafePortableBasenamesForWindows[] = {
+    FILE_PATH_LITERAL("con"),
+    FILE_PATH_LITERAL("con.zip"),
+    FILE_PATH_LITERAL("NUL"),
+    FILE_PATH_LITERAL("NUL.zip"),
+};
+
 static const base::FilePath::CharType* kSafePortableRelativePaths[] = {
     FILE_PATH_LITERAL("a/a"),
 #if defined(OS_WIN)
@@ -1409,4 +1416,18 @@ TEST(FilenameUtilTest, GenerateFileName) {
   }
 }
 
+TEST(FilenameUtilTest, IsReservedNameOnWindows) {
+  for (size_t i = 0; i < arraysize(kSafePortableBasenames); ++i) {
+    EXPECT_FALSE(IsReservedNameOnWindows(
+        base::FilePath(kSafePortableBasenames[i]).value()))
+        << kSafePortableBasenames[i];
+  }
+
+  for (size_t i = 0; i < arraysize(kUnsafePortableBasenamesForWindows); ++i) {
+    EXPECT_TRUE(IsReservedNameOnWindows(
+        base::FilePath(kUnsafePortableBasenamesForWindows[i]).value()))
+        << kUnsafePortableBasenamesForWindows[i];
+  }
+}
+
 }  // namespace net