Fix a path traversal issue in extension unpacking. Because of the sandbox, this was not exploitable, but still a good thing to fix.

Review URL: http://codereview.chromium.org/399063

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@32502 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 20 insertions, 5 deletions

diff --git a/chrome/common/zip.cc b/chrome/common/zip.cc
index 097f035..3d5e729 100644
--- a/chrome/common/zip.cc
+++ b/chrome/common/zip.cc
@@ -29,11 +29,6 @@ static bool ExtractCurrentFile(unzFile zip_file,
   if (filename_inzip[0] == '\0')
     return false;
 
-  // Check the filename here for directory traversal issues. In the name of
-  // simplicity and security, we might reject a valid filename such as "a..b"
-  if (strstr(filename_inzip, "..") != NULL)
-    return false;
-
   err = unzOpenCurrentFile(zip_file);
   if (err != UNZ_OK)
     return false;
@@ -45,6 +40,12 @@ static bool ExtractCurrentFile(unzFile zip_file,
 #elif defined(OS_POSIX)
   filename = filename_inzip;
 #endif
+
+  // Check the filename here for directory traversal issues. In the name of
+  // simplicity and security, we might reject a valid filename such as "a..b".
+  if (filename.find(FILE_PATH_LITERAL("..")) != FilePath::StringType::npos)
+    return false;
+
   SplitString(filename, '/', &filename_parts);
 
   FilePath dest_file(dest_dir);
diff --git a/chrome/common/zip_unittest.cc b/chrome/common/zip_unittest.cc
index ae43a09..57eabf4 100644
--- a/chrome/common/zip_unittest.cc
+++ b/chrome/common/zip_unittest.cc
@@ -110,6 +110,20 @@ TEST_F(ZipTest, UnzipEvil) {
   ASSERT_FALSE(file_util::PathExists(evil_file));
 }
 
+TEST_F(ZipTest, UnzipEvil2) {
+  ScopedTempDir dest_dir;
+  ASSERT_TRUE(dest_dir.CreateUniqueTempDir());
+
+  FilePath test_dir;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_dir));
+  test_dir = test_dir.AppendASCII("zip");
+  TestUnzipFile(FILE_PATH_LITERAL("evil_via_invalid_utf8.zip"), true, false);
+
+  FilePath evil_file = dest_dir.path();
+  evil_file = evil_file.AppendASCII("../evil.txt");
+  ASSERT_FALSE(file_util::PathExists(evil_file));
+}
+
 TEST_F(ZipTest, Zip) {
   FilePath src_dir;
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &src_dir));
diff --git a/chrome/test/data/zip/evil_via_invalid_utf8.zip b/chrome/test/data/zip/evil_via_invalid_utf8.zip
new file mode 100755
index 0000000..94b633e
--- /dev/null
+++ b/chrome/test/data/zip/evil_via_invalid_utf8.zip