Loosen permission on extension temp dir when a flag is used.

Issue 35198 can not be reproduced locally.  To enable users to do experiments, three command line flags are added to chrome:

--issue35198-crxdir-browser: Have the browser process create the directory in which the extension will be unzipped.

--issue35198-logging: Enable log messages from directory creation in the utility process to be moved to the browser process.

--issue35198-permission: Use the most permissive file permissions possible on the extension unpack directory.

BUG=35198
TEST=manual

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

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

7 files changed, 95 insertions, 12 deletions

diff --git a/base/file_util.h b/base/file_util.h
index 48f431f..f33e78b 100644
--- a/base/file_util.h
+++ b/base/file_util.h
@@ -267,7 +267,8 @@ bool CreateTemporaryFileInDir(const FilePath& dir,
 // new directory does not have the same name as an existing directory.
 bool CreateTemporaryDirInDir(const FilePath& base_dir,
                              const FilePath::StringType& prefix,
-                             FilePath* new_dir);
+                             FilePath* new_dir,
+                             bool lossen_permissions);
 
 // Create a new directory under TempPath. If prefix is provided, the new
 // directory name is in the format of prefixyyyy.
diff --git a/base/file_util_posix.cc b/base/file_util_posix.cc
index 9d738da..e868a41 100644
--- a/base/file_util_posix.cc
+++ b/base/file_util_posix.cc
@@ -428,7 +428,15 @@ static bool CreateTemporaryDirInDirImpl(const FilePath& base_dir,
 
 bool CreateTemporaryDirInDir(const FilePath& base_dir,
                              const FilePath::StringType& prefix,
-                             FilePath* new_dir) {
+                             FilePath* new_dir,
+                             bool loosen_permissions) {
+  // To understand crbug/35198, the ability to call this
+  // this function on windows while giving loose permissions
+  // to the resulting directory has been temporarily added.
+  // It should not be posible to call this function with
+  // loosen_permissions == true on non-windows plarforms.
+  CHECK(!loosen_permissions);
+
   FilePath::StringType mkdtemp_template = prefix;
   mkdtemp_template.append(FILE_PATH_LITERAL("XXXXXX"));
   return CreateTemporaryDirInDirImpl(base_dir, mkdtemp_template, new_dir);
diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc
index 39e5398..b0a1a8e 100644
--- a/base/file_util_unittest.cc
+++ b/base/file_util_unittest.cc
@@ -1569,7 +1569,8 @@ TEST_F(FileUtilTest, CreateNewTemporaryDirInDirTest) {
   ASSERT_TRUE(file_util::CreateTemporaryDirInDir(
                   test_dir_,
                   FILE_PATH_LITERAL("CreateNewTemporaryDirInDirTest"),
-                  &new_dir));
+                  &new_dir,
+                  false));
   EXPECT_TRUE(file_util::PathExists(new_dir));
   EXPECT_TRUE(test_dir_.IsParent(new_dir));
   EXPECT_TRUE(file_util::Delete(new_dir, false));
diff --git a/base/file_util_win.cc b/base/file_util_win.cc
index 8a15370..74db90cd 100644
--- a/base/file_util_win.cc
+++ b/base/file_util_win.cc
@@ -63,6 +63,56 @@ bool DevicePathToDriveLetterPath(const FilePath& device_path,
   return true;
 }
 
+// Build a security descriptor with the weakest possible file permissions.
+bool InitLooseSecurityDescriptor(SECURITY_ATTRIBUTES *sa,
+                                 SECURITY_DESCRIPTOR *sd) {
+  DWORD last_error;
+
+  if (!InitializeSecurityDescriptor(sd, SECURITY_DESCRIPTOR_REVISION)) {
+    last_error = GetLastError();
+    LOG(ERROR) << "InitializeSecurityDescriptor failed: GetLastError() = "
+               << last_error;
+    return false;
+  }
+
+  if (!SetSecurityDescriptorDacl(sd,
+                                 TRUE,  // bDaclPresent: Add one to |sd|.
+                                 NULL,  // pDacl: NULL means allow all access.
+                                 FALSE  // bDaclDefaulted: Not defaulted.
+                                 )) {
+    last_error = GetLastError();
+    LOG(ERROR) << "SetSecurityDescriptorDacl() failed: GetLastError() = "
+               << last_error;
+    return false;
+  }
+
+  if (!SetSecurityDescriptorGroup(sd,
+                                  NULL,  // pGroup: No no primary group.
+                                  FALSE  // bGroupDefaulted: Not defaulted.
+                                  )) {
+    last_error = GetLastError();
+    LOG(ERROR) << "SetSecurityDescriptorGroup() failed: GetLastError() = "
+               << last_error;
+    return false;
+  }
+
+  if (!SetSecurityDescriptorSacl(sd,
+                                 FALSE,  // bSaclPresent: No SACL.
+                                 NULL,
+                                 FALSE
+                                 )) {
+    last_error = GetLastError();
+    LOG(ERROR) << "SetSecurityDescriptorSacl() failed: GetLastError() = "
+               << last_error;
+    return false;
+  }
+
+  sa->nLength = sizeof(SECURITY_ATTRIBUTES);
+  sa->lpSecurityDescriptor = sd;
+  sa->bInheritHandle = TRUE;
+  return true;
+}
+
 }  // namespace
 
 std::wstring GetDirectoryFromPath(const std::wstring& path) {
@@ -550,7 +600,19 @@ bool CreateTemporaryFileInDir(const FilePath& dir,
 
 bool CreateTemporaryDirInDir(const FilePath& base_dir,
                              const FilePath::StringType& prefix,
-                             FilePath* new_dir) {
+                             FilePath* new_dir,
+                             bool loosen_permissions) {
+  SECURITY_ATTRIBUTES sa;
+  SECURITY_DESCRIPTOR sd;
+
+  LPSECURITY_ATTRIBUTES directory_security_attributes = NULL;
+  if (loosen_permissions) {
+    if (InitLooseSecurityDescriptor(&sa, &sd))
+      directory_security_attributes = &sa;
+    else
+      LOG(ERROR) << "Failed to init security attributes, fall back to NULL.";
+  }
+
   FilePath path_to_create;
   srand(static_cast<uint32>(time(NULL)));
 
@@ -565,7 +627,8 @@ bool CreateTemporaryDirInDir(const FilePath& base_dir,
     new_dir_name.append(IntToWString(rand() % kint16max));
 
     path_to_create = path_to_create.Append(new_dir_name);
-    if (::CreateDirectory(path_to_create.value().c_str(), NULL))
+    if (::CreateDirectory(path_to_create.value().c_str(),
+                          directory_security_attributes))
       break;
     count++;
   }
@@ -575,6 +638,7 @@ bool CreateTemporaryDirInDir(const FilePath& base_dir,
   }
 
   *new_dir = path_to_create;
+
   return true;
 }
 
@@ -584,7 +648,10 @@ bool CreateNewTempDirectory(const FilePath::StringType& prefix,
   if (!GetTempDir(&system_temp_dir))
     return false;
 
-  return CreateTemporaryDirInDir(system_temp_dir, prefix, new_temp_path);
+  return CreateTemporaryDirInDir(system_temp_dir,
+                                 prefix,
+                                 new_temp_path,
+                                 false);
 }
 
 bool CreateDirectory(const FilePath& full_path) {
diff --git a/base/scoped_temp_dir.cc b/base/scoped_temp_dir.cc
index c8ed9c6..c3fb00a 100644
--- a/base/scoped_temp_dir.cc
+++ b/base/scoped_temp_dir.cc
@@ -4,6 +4,7 @@
 
 #include "base/scoped_temp_dir.h"
 
+#include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/string_util.h"
@@ -26,16 +27,20 @@ bool ScopedTempDir::CreateUniqueTempDir() {
   return true;
 }
 
-bool ScopedTempDir::CreateUniqueTempDirUnderPath(const FilePath& base_path) {
+bool ScopedTempDir::CreateUniqueTempDirUnderPath(const FilePath& base_path,
+                                                 bool loose_permissions) {
   // If |path| does not exist, create it.
-  if (!file_util::CreateDirectory(base_path))
+  if (!file_util::CreateDirectory(base_path)) {
+    LOG(ERROR) << "Failed to create base directory " << base_path.value();
     return false;
+  }
 
-  // Create a new, uniquly named directory under |base_path|.
+  // Create a new, uniquely named directory under |base_path|.
   if (!file_util::CreateTemporaryDirInDir(
           base_path,
           FILE_PATH_LITERAL("scoped_dir_"),
-          &path_)) {
+          &path_,
+          loose_permissions)) {
     return false;
   }
   return true;
diff --git a/base/scoped_temp_dir.h b/base/scoped_temp_dir.h
index a5dca1e..a0708dc3 100644
--- a/base/scoped_temp_dir.h
+++ b/base/scoped_temp_dir.h
@@ -26,7 +26,8 @@ class ScopedTempDir {
   bool CreateUniqueTempDir();
 
   // Creates a unique directory under a given path, and takes ownership of it.
-  bool CreateUniqueTempDirUnderPath(const FilePath& path);
+  bool CreateUniqueTempDirUnderPath(const FilePath& path,
+                                    bool loose_permissions);
 
   // Takes ownership of directory at |path|, creating it if necessary.
   // Don't call multiple times unless Take() has been called first.
diff --git a/base/scoped_temp_dir_unittest.cc b/base/scoped_temp_dir_unittest.cc
index 4be0d07..e180119 100644
--- a/base/scoped_temp_dir_unittest.cc
+++ b/base/scoped_temp_dir_unittest.cc
@@ -65,7 +65,7 @@ TEST(ScopedTempDir, UniqueTempDirUnderPath) {
   FilePath test_path;
   {
     ScopedTempDir dir;
-    EXPECT_TRUE(dir.CreateUniqueTempDirUnderPath(base_path));
+    EXPECT_TRUE(dir.CreateUniqueTempDirUnderPath(base_path, false));
     test_path = dir.path();
     EXPECT_TRUE(file_util::DirectoryExists(test_path));
     EXPECT_TRUE(base_path.IsParent(test_path));