Fix extensions install procedure to use the user's temp directory for unzipping.

Extensions should use the users temp dir instead of temp dir inside the profile because if we have roamed profile writing there from the sandbox will fail.

There is a reason we don't copy directly to the target too:
this has been done like that in order to ensure that no extensions is copied only partially to its final location when the profile and temp are on separate volumes.

BUG=84045
TEST=manual

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

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

2 files changed, 89 insertions, 6 deletions

diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
index 4178bda..35414b4 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
@@ -108,6 +108,57 @@ void RecordSuccessfulUnpackTimeHistograms(
   }
 }
 
+// Work horse for FindWritableTempLocation. Creates a temp file in the folder
+// and uses NormalizeFilePath to check if the path is junction free.
+bool VerifyJunctionFreeLocation(FilePath* temp_dir) {
+  if (temp_dir->empty())
+    return false;
+
+  FilePath temp_file;
+  if (!file_util::CreateTemporaryFileInDir(*temp_dir, &temp_file)) {
+    LOG(ERROR) << temp_dir->value() << " is not writable";
+    return false;
+  }
+  // NormalizeFilePath requires a non-empty file, so write some data.
+  // If you change the exit points of this function please make sure all
+  // exit points delete this temp file!
+  file_util::WriteFile(temp_file, ".", 1);
+
+  FilePath normalized_temp_file;
+  bool normalized =
+      file_util::NormalizeFilePath(temp_file, &normalized_temp_file);
+  if (!normalized) {
+    // If |temp_file| contains a link, the sandbox will block al file system
+    // operations, and the install will fail.
+    LOG(ERROR) << temp_dir->value() << " seem to be on remote drive.";
+  } else {
+    *temp_dir = normalized_temp_file.DirName();
+  }
+  // Clean up the temp file.
+  file_util::Delete(temp_file, false);
+
+  return normalized;
+}
+
+// This function tries to find a location for unpacking the extension archive
+// that is writable and does not lie on a shared drive so that the sandboxed
+// unpacking process can write there. If no such location exists we can not
+// proceed and should fail.
+// The result will be written to |temp_dir|. The function will write to this
+// parameter even if it returns false.
+bool FindWritableTempLocation(FilePath* temp_dir) {
+  PathService::Get(base::DIR_TEMP, temp_dir);
+  if (VerifyJunctionFreeLocation(temp_dir))
+    return true;
+  *temp_dir = extension_file_util::GetUserDataTempDir();
+  if (VerifyJunctionFreeLocation(temp_dir))
+    return true;
+  // Neither paths is link free chances are good installation will fail.
+  LOG(ERROR) << "Both the %TEMP% folder and the profile seem to be on "
+             << "remote drives or read-only. Installation can not complete!";
+  return false;
+}
+
 }  // namespace
 
 SandboxedExtensionUnpacker::SandboxedExtensionUnpacker(
@@ -125,8 +176,8 @@ SandboxedExtensionUnpacker::SandboxedExtensionUnpacker(
 bool SandboxedExtensionUnpacker::CreateTempDirectory() {
   CHECK(BrowserThread::GetCurrentThreadIdentifier(&thread_identifier_));
 
-  FilePath user_data_temp_dir = extension_file_util::GetUserDataTempDir();
-  if (user_data_temp_dir.empty()) {
+  FilePath temp_dir;
+  if (!FindWritableTempLocation(&temp_dir)) {
     ReportFailure(
         COULD_NOT_GET_TEMP_DIRECTORY,
         l10n_util::GetStringFUTF16(
@@ -135,7 +186,7 @@ bool SandboxedExtensionUnpacker::CreateTempDirectory() {
     return false;
   }
 
-  if (!temp_dir_.CreateUniqueTempDirUnderPath(user_data_temp_dir)) {
+  if (!temp_dir_.CreateUniqueTempDirUnderPath(temp_dir)) {
     ReportFailure(
         COULD_NOT_CREATE_TEMP_DIRECTORY,
         l10n_util::GetStringFUTF16(
diff --git a/chrome/common/extensions/extension_file_util.cc b/chrome/common/extensions/extension_file_util.cc
index b0a1935..ca31d83 100644
--- a/chrome/common/extensions/extension_file_util.cc
+++ b/chrome/common/extensions/extension_file_util.cc
@@ -57,6 +57,35 @@ FilePath InstallExtension(const FilePath& unpacked_source_dir,
       return FilePath();
   }
 
+  FilePath profile_temp_dir = GetUserDataTempDir();
+  // Move the extracted extension to a temp folder under the profile which will
+  // then be moved to the final destination to ensure integrity of the installed
+  // extension. The first move is actually a copy+delete to ensure proper
+  // behavor in case we are moving a folder inside another folder on the same
+  // level because Move will attempt rename in this case instead of proper move.
+  // PLEASE NOTE: This issue has been observed in extension unit tests that try
+  // to install user exnteions (not crx files but unpacked ones) from subfolder
+  // of the temp folder. In that case a move will only rename the folder insted
+  // of miving it into the destination folder as expected. That is the reason we
+  // do copy+delete instead of a plain delete here! It can happen in the wild
+  // with say autounpacked archive going to the temp folder and the user tries
+  // to install it from there.
+  ScopedTempDir extension_temp_dir;
+  if (profile_temp_dir.empty() ||
+      !extension_temp_dir.CreateUniqueTempDirUnderPath(profile_temp_dir)) {
+    LOG(ERROR) << "Creating of temp dir under in the profile failed.";
+    return FilePath();
+  }
+  if (!file_util::CopyDirectory(unpacked_source_dir,
+                                extension_temp_dir.path(), true)) {
+    LOG(ERROR) << "Moving extension from : " << unpacked_source_dir.value()
+               << " to : " << extension_temp_dir.path().value() << " failed.";
+    return FilePath();
+  }
+  file_util::Delete(unpacked_source_dir, true);
+  FilePath crx_temp_source =
+      extension_temp_dir.path().Append(unpacked_source_dir.BaseName());
+
   // Try to find a free directory. There can be legitimate conflicts in the case
   // of overinstallation of the same version.
   const int kMaxAttempts = 100;
@@ -70,13 +99,16 @@ FilePath InstallExtension(const FilePath& unpacked_source_dir,
   }
 
   if (version_dir.empty()) {
-    DLOG(ERROR) << "Could not find a home for extension " << id << " with "
-                << "version " << version << ".";
+    LOG(ERROR) << "Could not find a home for extension " << id << " with "
+               << "version " << version << ".";
     return FilePath();
   }
 
-  if (!file_util::Move(unpacked_source_dir, version_dir))
+  if (!file_util::Move(crx_temp_source, version_dir)) {
+    LOG(ERROR) << "Installing extension from : " << crx_temp_source.value()
+               << " into : " << version_dir.value() << " failed.";
     return FilePath();
+  }
 
   return version_dir;
 }