crash-reporter: Avoid writing through symlinks.
BUG=7987
TEST=bvts
Change-Id: I875adeb5073936e790beb93f6a223a1642131cbd
Review URL: http://codereview.chromium.org/4603001
diff --git a/crash_reporter/crash_collector.cc b/crash_reporter/crash_collector.cc
index 1e2cd1a..7abe2a5 100644
--- a/crash_reporter/crash_collector.cc
+++ b/crash_reporter/crash_collector.cc
@@ -5,11 +5,15 @@
#include "crash-reporter/crash_collector.h"
#include <dirent.h>
+#include <fcntl.h> // For file creation modes.
#include <pwd.h> // For struct passwd.
#include <sys/types.h> // for mode_t.
+#include <sys/wait.h> // For waitpid.
+#include <unistd.h> // For execv and fork.
#include <set>
+#include "base/eintr_wrapper.h"
#include "base/file_util.h"
#include "base/logging.h"
#include "base/string_util.h"
@@ -60,6 +64,76 @@
logger_ = logger;
}
+int CrashCollector::WriteNewFile(const FilePath &filename,
+ const char *data,
+ int size) {
+ int fd = HANDLE_EINTR(open(filename.value().c_str(),
+ O_CREAT | O_WRONLY | O_TRUNC | O_EXCL, 0666));
+ if (fd < 0) {
+ return -1;
+ }
+
+ int rv = file_util::WriteFileDescriptor(fd, data, size);
+ HANDLE_EINTR(close(fd));
+ return rv;
+}
+
+int CrashCollector::ForkExecAndPipe(std::vector<const char *> &arguments,
+ const char *output_file) {
+ // Copy off a writeable version of arguments.
+ scoped_array<char*> argv(new char *[arguments.size() + 1]);
+ int total_args_size = 0;
+ for (size_t i = 0; i < arguments.size(); ++i) {
+ if (arguments[i] == NULL) {
+ logger_->LogError("Bad parameter");
+ return -1;
+ }
+ total_args_size += strlen(arguments[i]) + 1;
+ }
+ scoped_array<char> buffer(new char[total_args_size]);
+ char *buffer_pointer = &buffer[0];
+
+ for (size_t i = 0; i < arguments.size(); ++i) {
+ argv[i] = buffer_pointer;
+ strcpy(buffer_pointer, arguments[i]);
+ buffer_pointer += strlen(arguments[i]);
+ *buffer_pointer = '\0';
+ ++buffer_pointer;
+ }
+ argv[arguments.size()] = NULL;
+
+ int pid = fork();
+ if (pid < 0) {
+ logger_->LogError("Fork failed: %d", errno);
+ return -1;
+ }
+
+ if (pid == 0) {
+ int output_handle = HANDLE_EINTR(creat(output_file, 0700));
+ if (output_handle < 0) {
+ logger_->LogError("Could not create %s: %d", output_file, errno);
+ // Avoid exit() to avoid atexit handlers from parent.
+ _exit(127);
+ }
+ dup2(output_handle, 1);
+ dup2(output_handle, 2);
+ execv(argv[0], &argv[0]);
+ logger_->LogError("Exec failed: %d", errno);
+ _exit(127);
+ }
+
+ int status = 0;
+ if (HANDLE_EINTR(waitpid(pid, &status, 0)) < 0) {
+ logger_->LogError("Problem waiting for pid: %d", errno);
+ return -1;
+ }
+ if (!WIFEXITED(status)) {
+ logger_->LogError("Process did not exit normally: %d", status);
+ return -1;
+ }
+ return WEXITSTATUS(status);
+}
+
std::string CrashCollector::Sanitize(const std::string &name) {
std::string result = name;
for (size_t i = 0; i < name.size(); ++i) {
@@ -291,7 +365,10 @@
version.c_str(),
payload_path.c_str(),
payload_size);
- if (!file_util::WriteFile(meta_path, meta_data.c_str(), meta_data.size())) {
+ // We must use WriteNewFile instead of file_util::WriteFile as we
+ // do not want to write with root access to a symlink that an attacker
+ // might have created.
+ if (WriteNewFile(meta_path, meta_data.c_str(), meta_data.size()) < 0) {
logger_->LogError("Unable to write %s", meta_path.value().c_str());
}
}