summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--base/file_util.h5
-rw-r--r--base/file_util_posix.cc8
-rw-r--r--base/file_util_unittest.cc1
-rw-r--r--base/file_util_win.cc71
-rw-r--r--base/scoped_temp_dir.cc11
-rw-r--r--base/scoped_temp_dir.h3
-rw-r--r--base/scoped_temp_dir_unittest.cc2
-rw-r--r--chrome/browser/extensions/sandboxed_extension_unpacker.cc24
-rw-r--r--chrome/browser/utility_process_host.cc3
-rw-r--r--chrome/common/chrome_switches.cc7
-rw-r--r--chrome/common/chrome_switches.h3
-rw-r--r--chrome/common/extensions/extension_unpacker.cc17
12 files changed, 142 insertions, 13 deletions
diff --git a/base/file_util.h b/base/file_util.h
index 48f431f..6277748 100644
--- a/base/file_util.h
+++ b/base/file_util.h
@@ -265,8 +265,13 @@ bool CreateTemporaryFileInDir(const FilePath& dir,
// Create a directory within another directory.
// Extra characters will be appended to |name_tmpl| to ensure that the
// new directory does not have the same name as an existing directory.
+// If |loosen_permissions| is true, the new directory will be readable
+// and writable to all users on windows. It is ignored on other platforms.
+// |loosen_permissions| exists to allow debugging of crbug/35198, and will
+// be removed when the issue is understood.
bool CreateTemporaryDirInDir(const FilePath& base_dir,
const FilePath::StringType& prefix,
+ bool loosen_permissions,
FilePath* new_dir);
// Create a new directory under TempPath. If prefix is provided, the new
diff --git a/base/file_util_posix.cc b/base/file_util_posix.cc
index 9d738da..434c859 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,
+ bool loosen_permissions,
FilePath* new_dir) {
+ // 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 possible to call this function with
+ // loosen_permissions == true on non-windows platforms.
+ DCHECK(!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..4827f59 100644
--- a/base/file_util_unittest.cc
+++ b/base/file_util_unittest.cc
@@ -1569,6 +1569,7 @@ TEST_F(FileUtilTest, CreateNewTemporaryDirInDirTest) {
ASSERT_TRUE(file_util::CreateTemporaryDirInDir(
test_dir_,
FILE_PATH_LITERAL("CreateNewTemporaryDirInDirTest"),
+ false,
&new_dir));
EXPECT_TRUE(file_util::PathExists(new_dir));
EXPECT_TRUE(test_dir_.IsParent(new_dir));
diff --git a/base/file_util_win.cc b/base/file_util_win.cc
index 8a15370..74b9406 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,
+ bool loosen_permissions,
FilePath* new_dir) {
+ 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,
+ false,
+ new_temp_path);
}
bool CreateDirectory(const FilePath& full_path) {
diff --git a/base/scoped_temp_dir.cc b/base/scoped_temp_dir.cc
index c8ed9c6..28f59bf 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,15 +27,19 @@ 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_"),
+ loose_permissions,
&path_)) {
return false;
}
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));
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
index eda5e3f..c087649 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
@@ -42,8 +42,18 @@ void SandboxedExtensionUnpacker::Start() {
// file IO on.
CHECK(ChromeThread::GetCurrentThreadIdentifier(&thread_identifier_));
+ // To understand crbug/35198, allow users who can reproduce the bug
+ // to loosen permissions on the scoped directory.
+ bool loosen_permissions = false;
+#if defined (OS_WIN)
+ loosen_permissions = CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kIssue35198Permission);
+ LOG(INFO) << "loosen_permissions = " << loosen_permissions;
+#endif
+
// Create a temporary directory to work in.
- if (!temp_dir_.CreateUniqueTempDirUnderPath(temp_path_)) {
+ if (!temp_dir_.CreateUniqueTempDirUnderPath(temp_path_,
+ loosen_permissions)) {
ReportFailure("Could not create temporary directory.");
return;
}
@@ -52,6 +62,15 @@ void SandboxedExtensionUnpacker::Start() {
extension_root_ = temp_dir_.path().AppendASCII(
extension_filenames::kTempExtensionName);
+ // To understand crbug/35198, allow users who can reproduce the bug to
+ // create the unpack directory in the browser process.
+ bool crxdir_in_browser = CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kIssue35198CrxDirBrowser);
+ LOG(INFO) << "crxdir_in_browser = " << crxdir_in_browser;
+ if (crxdir_in_browser && !file_util::CreateDirectory(extension_root_)) {
+ LOG(ERROR) << "Failed to create directory " << extension_root_.value();
+ }
+
// Extract the public key and validate the package.
if (!ValidateSignature())
return; // ValidateSignature() already reported the error.
@@ -69,9 +88,6 @@ void SandboxedExtensionUnpacker::Start() {
// the link will cause file system access outside the sandbox path.
FilePath normalized_crx_path;
if (!file_util::NormalizeFilePath(temp_crx_path, &normalized_crx_path)) {
- // TODO(skerner): Remove this logging once crbug/13044 is fixed.
- // This bug is starred by many users who have some kind of link.
- // If NormalizeFilePath() fails we want to see it in the logs they send.
LOG(ERROR) << "Could not get the normalized path of "
<< temp_crx_path.value();
normalized_crx_path = temp_crx_path;
diff --git a/chrome/browser/utility_process_host.cc b/chrome/browser/utility_process_host.cc
index be5397e..6dbf3ba 100644
--- a/chrome/browser/utility_process_host.cc
+++ b/chrome/browser/utility_process_host.cc
@@ -100,6 +100,9 @@ bool UtilityProcessHost::StartProcess(const FilePath& exposed_dir) {
cmd_line->AppendSwitch(switches::kEnableExperimentalExtensionApis);
}
+ if (browser_command_line.HasSwitch(switches::kIssue35198ExtraLogging))
+ cmd_line->AppendSwitch(switches::kIssue35198ExtraLogging);
+
#if defined(OS_POSIX)
// TODO(port): Sandbox this on Linux. Also, zygote this to work with
// Linux updating.
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 3e191f3..e89f57e 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -529,6 +529,13 @@ const char kInternalNaCl[] = "internal-nacl";
// Runs a trusted Pepper plugin inside the renderer process.
const char kInternalPepper[] = "internal-pepper";
+// The following flags allow users who can reproduce crbug/35198
+// to enable extra logging and behaviors. They will be removed once
+// the issue is fixed.
+const char kIssue35198CrxDirBrowser[] = "issue35198-crxdir-browser";
+const char kIssue35198ExtraLogging[] = "issue35198-logging";
+const char kIssue35198Permission[] = "issue35198-permission";
+
// Specifies the flags passed to JS engine
const char kJavaScriptFlags[] = "js-flags";
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 2348d92..183c3a9 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -160,6 +160,9 @@ extern const char kInstallerTestClean[];
extern const char kInstallerTestForce[];
extern const char kInternalNaCl[];
extern const char kInternalPepper[];
+extern const char kIssue35198CrxDirBrowser[];
+extern const char kIssue35198ExtraLogging[];
+extern const char kIssue35198Permission[];
extern const char kJavaScriptFlags[];
extern const char kLoadExtension[];
extern const char kLoadPlugin[];
diff --git a/chrome/common/extensions/extension_unpacker.cc b/chrome/common/extensions/extension_unpacker.cc
index 599aa11..64ff857 100644
--- a/chrome/common/extensions/extension_unpacker.cc
+++ b/chrome/common/extensions/extension_unpacker.cc
@@ -4,6 +4,7 @@
#include "chrome/common/extensions/extension_unpacker.h"
+#include "base/command_line.h"
#include "base/file_util.h"
#include "base/scoped_handle.h"
#include "base/scoped_temp_dir.h"
@@ -11,6 +12,7 @@
#include "base/thread.h"
#include "base/values.h"
#include "net/base/file_stream.h"
+#include "chrome/common/chrome_switches.h"
#include "chrome/common/common_param_traits.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_constants.h"
@@ -142,12 +144,23 @@ bool ExtensionUnpacker::Run() {
extension_path_.DirName().AppendASCII(filenames::kTempExtensionName);
#if defined(OS_WIN)
+ // To understand crbug/35198, allow users who can reproduce the issue
+ // to enable extra logging while unpacking.
+ bool extra_logging = CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kIssue35198ExtraLogging);
+ LOG(INFO) << "Extra logging for issue 35198: " << extra_logging;
+
std::ostringstream log_stream;
std::string dir_string = WideToUTF8(temp_install_dir_.value());
log_stream << kCouldNotCreateDirectoryError << dir_string << std::endl;
+
if (!file_util::CreateDirectoryExtraLogging(temp_install_dir_, log_stream)) {
- log_stream.flush();
- SetError(log_stream.str());
+ if (extra_logging) {
+ log_stream.flush();
+ SetError(log_stream.str());
+ } else {
+ SetError(kCouldNotCreateDirectoryError + dir_string);
+ }
return false;
}
#else
generated by cgit v1.1 at 2025-01-29 15:02:54 +0000