Exclude exe files while unzipping CRXes

BUG=468355

Review-Url: https://codereview.chromium.org/2321823002
Cr-Commit-Position: refs/heads/master@{#417494}

chrome/browser/extensions/crx_installer_browsertest.cc

@@ -345,6 +345,15 @@ IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTestWithExperimentalApis,
       test_data_dir_.AppendASCII("minimal_platform_app.crx"), 1));
 }
 
+IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTest, BlockedFileTypes) {
+  const Extension* extension =
+      InstallExtension(test_data_dir_.AppendASCII("blocked_file_types.crx"), 1);
+  EXPECT_TRUE(base::PathExists(extension->path().AppendASCII("test.html")));
+  EXPECT_TRUE(base::PathExists(extension->path().AppendASCII("test.nexe")));
+  EXPECT_FALSE(base::PathExists(extension->path().AppendASCII("test1.EXE")));
+  EXPECT_FALSE(base::PathExists(extension->path().AppendASCII("test2.exe")));
+}
+
 IN_PROC_BROWSER_TEST_F(ExtensionCrxInstallerTest, PackAndInstallExtension) {
   if (!FeatureSwitch::easy_off_store_install()->IsEnabled())
     return;

chrome/test/data/extensions/blocked_file_types.pem

+-----BEGIN PRIVATE KEY-----
+MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQDGrF6hJzluvwWL
+EtjWigfpkWew6PN403vijTQ372B+gij8oGcv2F22NPZMefhhnd47nFmfysrr9bBg
+vxyZANwI4ZQhapwLnaOf/vT0sp2kWfnf+gGOKHI6b8dJWKr7VGR989GJUhn9wVqf
+hW5yXw4dAnsTkMAVnNRylqd8bHseCV2yUDtJoheNw/qr3JROqRJD5Y2uSsTyjccO
+PCJirGpfUX2Hdph9WxvlAC/u0KVG/Hmc1wO7OBxIr/futHpjvaIyZSQEyF2OILsI
+sjK2uAjp16vyC2SeMMKTcVg4J0dMjdS0FaqY3oGA9RsgHIVjFopE/+16YH33LwHX
+0LucXj/jAgMBAAECggEANfN3GyIwuX9MPREYK2J+dZIT63nHLEeZ+9LGnG338ULd
+aIUSoG6MPD+zpt70cWXShLOXrwn+AwfMJumfFXmjRxIjDNQLnifn2Dn/QTgEES6R
+6Z3VsEoRgwb5wlQ+0lN/mqiMfVL7Veqc9nbgoFRp27zhuVV0pCnbk2aQAvzejjAY
+Sha8CFDKp71sfItWt0cKxvRLZtBxD1a8BEHb/o4Dkr1I1tz94+5NtlpIrWay25dn
+QHCXTvqhA6EU4ZSzq7dKcTunKG9lMyQ+r63GnNFgjnJjmE5nSMMu8zKG3XuuFRLw
+T59385ztJEZC+dltG7ulpOT4xSFHiPqgXLriCYJtAQKBgQDtOVydTPpDiujnItcz
+0qTjAGi04p3khTpM22Xxn8l2oJQyrlQk18yEfUDrlyh2VR/AbkbugXoTWv3SchgT
+9DGt89woL8fZ9D5voyuxoiUZzU6iYh1F7W3goLr2QGjQ+UBBkCvL2pnimTT9CuwH
+cv9ztpJ0kg+Qr+YSUn6Wp6zqYwKBgQDWZeRTISp0KPHUSYuCE+Bd1yv47cwmd9Rm
+Z9wKlDkoAd+psnUNABqzeWlKaswY/WxtHoMVPwIHvsltcU+OB7S3opPaTPXYl1Ev
+yz7dkFZ/5Ll0bb0EbA3uLwxfPAxCcbnP677Wg5/ZtLzG1unSX0zD8/zCtm4eI4Iq
+m2ISQBqMgQKBgChtqzP/p3MVXGXv0R862fD5495X9LK6ZdotVe1c3vfBnCAbB6oT
+wP4fXo4H/9RS44tm/92r4UMlMbbIzQOxjTnpIm8S3Ytujal29H5cdNg13h3cfMKr
+GRu8KMDI71QV9nZ1/JDMW4d7tsUdNGZ8mb59se396nstDxe2SW6het2jAoGAJDK1
+1xc9bLlRAy+bn02tzwo6QWAc3M3UjamxbBPUhp2yXptjydsfpvezwD3B0eJPwSiq
+pxapSBd9OL4t2WQRrgOEBRKDW5tIuNIO/ieVgyhh63dvt2RrAbDt3NJtm3ZKEgHS
+ibNVdNNgIA4/EibG9jqmjVVxyCLkoj7z0vMByIECgYBitbW69gUZTmdJeEgfC/oR
+41J06ZMekbx1TxIj0cHPGnU3ArDIhqK9tInYhPJfYHoFxcC2I1k+EHNXL2zIog6y
+QS9ax/LI1/SyLPMqZn0PN6HG5el8Q5qeBUQyHS043y6MS8oj1b6Oqpqk82pibeba
+U+yuALI1IDNUc1Pw6iRUFw==
+-----END PRIVATE KEY-----

extensions/utility/unpacker.cc

@@ -115,6 +115,12 @@ Unpacker::Unpacker(const base::FilePath& working_dir,
 Unpacker::~Unpacker() {
 }
 
+// static
+bool Unpacker::ShouldExtractFile(const base::FilePath& file_path) {
+  return !base::FilePath::CompareEqualIgnoreCase(file_path.FinalExtension(),
+                                                 FILE_PATH_LITERAL(".exe"));
+}
+
 std::unique_ptr<base::DictionaryValue> Unpacker::ReadManifest() {
   base::FilePath manifest_path = extension_dir_.Append(kManifestFilename);
   if (!base::PathExists(manifest_path)) {

extensions/utility/unpacker.h

@@ -34,6 +34,9 @@ class Unpacker {
            int creation_flags);
   ~Unpacker();
 
+  // Returns true if the given base::FilePath should be unzipped.
+  static bool ShouldExtractFile(const base::FilePath& file_path);
+
   // Runs the processing steps for the extension. On success, this returns true
   // and the decoded images will be in a file at
   // |working_dir|/kDecodedImagesFilename and the decoded messages will be in a

extensions/utility/unpacker_unittest.cc

@@ -196,4 +196,38 @@ TEST_F(UnpackerTest, ImageDecodingError) {
       << unpacker_->error_message() << "\"";
 }
 
+base::FilePath CreateEmptyTestFile(const base::FilePath& test_dir,
+                                   const base::FilePath& file_path) {
+  base::FilePath test_file(test_dir.Append(file_path));
+  base::FilePath temp_file;
+  EXPECT_TRUE(base::CreateTemporaryFileInDir(test_dir, &temp_file));
+  EXPECT_TRUE(base::Move(temp_file, test_file));
+  return test_file;
+}
+
+TEST_F(UnpackerTest, BlockedFileTypes) {
+  const struct {
+    const base::FilePath::CharType* input;
+    bool expected;
+  } cases[] = {
+      {FILE_PATH_LITERAL("foo"), true},
+      {FILE_PATH_LITERAL("foo.nexe"), true},
+      {FILE_PATH_LITERAL("foo.dll"), true},
+      {FILE_PATH_LITERAL("foo.jpg.exe"), false},
+      {FILE_PATH_LITERAL("foo.exe"), false},
+      {FILE_PATH_LITERAL("foo.EXE"), false},
+  };
+
+  base::ScopedTempDir temp_dir;
+  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+
+  for (size_t i = 0; i < arraysize(cases); ++i) {
+    base::FilePath input(cases[i].input);
+    base::FilePath test_file(CreateEmptyTestFile(temp_dir.path(), input));
+    bool observed = Unpacker::ShouldExtractFile(test_file);
+    EXPECT_EQ(cases[i].expected, observed) << "i: " << i
+                                           << ", input: " << test_file.value();
+  }
+}
+
 }  // namespace extensions

extensions/utility/utility_handler.cc

@@ -81,7 +81,11 @@ void UtilityHandler::OnParseUpdateManifest(const std::string& xml) {
 
 void UtilityHandler::OnUnzipToDir(const base::FilePath& zip_path,
                                   const base::FilePath& dir) {
-  if (!zip::Unzip(zip_path, dir)) {
+  const base::Callback<bool(const base::FilePath&)>& filter_cb =
+      base::Bind(&Unpacker::ShouldExtractFile);
+  // TODO(crbug.com/645263): This silently ignores blocked file types.
+  //                         Add install warnings.
+  if (!zip::UnzipWithFilterCallback(zip_path, dir, filter_cb)) {
     Send(new ExtensionUtilityHostMsg_UnzipToDir_Failed(
         std::string(kExtensionHandlerUnzipError)));
   } else {

third_party/zlib/google/zip.cc

@@ -94,6 +94,13 @@ bool ExcludeHiddenFilesFilter(const base::FilePath& file_path) {
 namespace zip {
 
 bool Unzip(const base::FilePath& src_file, const base::FilePath& dest_dir) {
+  return UnzipWithFilterCallback(src_file, dest_dir,
+                                 base::Bind(&ExcludeNoFilesFilter));
+}
+
+bool UnzipWithFilterCallback(const base::FilePath& src_file,
+                             const base::FilePath& dest_dir,
+                             const FilterCallback& filter_cb) {
   ZipReader reader;
   if (!reader.Open(src_file)) {
     DLOG(WARNING) << "Failed to open " << src_file.value();
@@ -109,11 +116,17 @@ bool Unzip(const base::FilePath& src_file, const base::FilePath& dest_dir) {
                     << reader.current_entry_info()->file_path().value();
       return false;
     }
-    if (!reader.ExtractCurrentEntryIntoDirectory(dest_dir)) {
-      DLOG(WARNING) << "Failed to extract "
+    if (filter_cb.Run(reader.current_entry_info()->file_path())) {
+      if (!reader.ExtractCurrentEntryIntoDirectory(dest_dir)) {
+        DLOG(WARNING) << "Failed to extract "
+                      << reader.current_entry_info()->file_path().value();
+        return false;
+      }
+    } else {
+      DLOG(WARNING) << "Skipped file "
                     << reader.current_entry_info()->file_path().value();
-      return false;
     }
+
     if (!reader.AdvanceToNextEntry()) {
       DLOG(WARNING) << "Failed to advance to the next file";
       return false;

third_party/zlib/google/zip.h

@@ -40,6 +40,14 @@ bool ZipFiles(const base::FilePath& src_dir,
               int dest_fd);
 #endif  // defined(OS_POSIX)
 
+// Unzip the contents of zip_file into dest_dir.
+// For each file in zip_file, include it only if the callback |filter_cb|
+// returns true. Otherwise omit it.
+typedef base::Callback<bool(const base::FilePath&)> FilterCallback;
+bool UnzipWithFilterCallback(const base::FilePath& zip_file,
+                             const base::FilePath& dest_dir,
+                             const FilterCallback& filter_cb);
+
 // Unzip the contents of zip_file into dest_dir.
 bool Unzip(const base::FilePath& zip_file, const base::FilePath& dest_dir);