File manager: support for zipping selected files.

chrome/utility/chrome_content_utility_client.cc

Index: chrome/utility/chrome_content_utility_client.cc
diff --git a/chrome/utility/chrome_content_utility_client.cc b/chrome/utility/chrome_content_utility_client.cc
index 8e6a69ddb2146f028fcf53ba313e5d47b1397d4d..b2fb974ba5024f82a3422f2e7a3006389cecb558 100644
--- a/chrome/utility/chrome_content_utility_client.cc
+++ b/chrome/utility/chrome_content_utility_client.cc
@@ -21,6 +21,7 @@
 #include "chrome/common/extensions/unpacker.h"
 #include "chrome/common/extensions/update_manifest.h"
 #include "chrome/common/web_resource/web_resource_unpacker.h"
+#include "chrome/common/zip.h"
 #include "chrome/utility/profile_import_handler.h"
 #include "content/public/utility/utility_thread.h"
 #include "printing/backend/print_backend.h"
@@ -88,6 +89,7 @@ bool ChromeContentUtilityClient::OnMessageReceived(
     IPC_MESSAGE_HANDLER(ChromeUtilityMsg_ParseJSON, OnParseJSON)
     IPC_MESSAGE_HANDLER(ChromeUtilityMsg_GetPrinterCapsAndDefaults,
                         OnGetPrinterCapsAndDefaults)
+    IPC_MESSAGE_HANDLER(ChromeUtilityMsg_CreateZipFile, OnCreateZipFile)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
 
@@ -186,6 +188,17 @@ void ChromeContentUtilityClient::OnDecodeImageBase64(
   OnDecodeImage(decoded_vector);
 }
 
+void ChromeContentUtilityClient::OnCreateZipFile(
+    const FilePath& src_dir,
+    const std::vector<FilePath>& src_relative_paths,
+    const FilePath& dest_file) {
+  if (zip::ZipFiles(src_dir, src_relative_paths, dest_file))

[palmer, 2012/11/13 20:43:40]

As it is, this function looks very dangerous.

zip::ZipFiles does no validation of the pathnames. As you say in
chrome/common/zip.cc,

178   // TODO(hshi): check that |src_relative_paths| does not contain
|dest_file|.

I'd also add that you should enforce that src_relative_paths are, in fact,
relative to src_dir.

Additionally, we need to make sure this is not an arbitrary file-write primitive
(e.g. one that overwrites existing, believed-trustworthy zip files, or any other
files).

This message-receiving code should determine |dest_file| itself; or,
alternately, validate |dest_file| strictly and then incorporate it into the
final pathanme as a suggestion from the caller. For example (in pseudocode):

    dest_file = strip_non_ascii(dest_file);
    dest_file = basename(dest_file);
    dest_file = strip_extension(dest_file);
    dest_file = strip_spaces(dest_file);
    dest_file = kZipFilesParent + kDirectorySeparator + dest_file + ".zip";

It should probably pass zip::ZipFiles an open HANDLE or file descriptor, rather
than a pathname, to avoid race conditions and name collisions. That probably
requires a new function overload for zip::ZipFiles.

    dest_descriptor = open(dest_file, O_CREAT|O_EXCL|O_TRUNC|O_WRONLY, 0644); 
// And whatever is appropriate for Windows

Similarly, there should be something like:

    foreach (pathname in src_relative_paths) {
        pathname = normalize_pathname(pathname);  // No "../" tricks, et c.
        if (is_absolute_path(pathname))
            fail();
        validated_src_paths.push_back(pathname);
    }

    zip::ZipFiles(src_dir, validated_src_paths, dest_descriptor);

+    Send(new ChromeUtilityHostMsg_CreateZipFile_Succeeded());
+  else
+    Send(new ChromeUtilityHostMsg_CreateZipFile_Failed());
+  content::UtilityThread::Get()->ReleaseProcessIfNeeded();
+}
+
 void ChromeContentUtilityClient::OnRenderPDFPagesToMetafile(
     base::PlatformFile pdf_file,
     const FilePath& metafile_path,