Sanitize dumped stacks to remove data that may be identifiable.

In order to sanitize the stack contents we erase any pointer-aligned
word that could not be interpreted as a pointer into one of the
processes' memory mappings, or a small integer (+/-4096).

This still retains enough information to unwind stack frames, and also
to recover some register values.

BUG=682278

Change-Id: I541a13b2e92a9d1aea2c06a50bd769a9e25601d3
Reviewed-on: https://chromium-review.googlesource.com/430050
Reviewed-by: Robert Sesek <rsesek@chromium.org>

3 files changed, 96 insertions, 5 deletions

diff --git a/src/client/linux/microdump_writer/microdump_writer.cc b/src/client/linux/microdump_writer/microdump_writer.cc
index 8109a981..341d7f5c 100644
--- a/src/client/linux/microdump_writer/microdump_writer.cc
+++ b/src/client/linux/microdump_writer/microdump_writer.cc
@@ -134,6 +134,7 @@ class MicrodumpWriter {
                   const MappingList& mappings,
                   bool skip_dump_if_principal_mapping_not_referenced,
                   uintptr_t address_within_principal_mapping,
+                  bool sanitize_stack,
                   const MicrodumpExtraInfo& microdump_extra_info,
                   LinuxDumper* dumper)
       : ucontext_(context ? &context->context : NULL),
@@ -145,6 +146,7 @@ class MicrodumpWriter {
         skip_dump_if_principal_mapping_not_referenced_(
             skip_dump_if_principal_mapping_not_referenced),
         address_within_principal_mapping_(address_within_principal_mapping),
+        sanitize_stack_(sanitize_stack),
         microdump_extra_info_(microdump_extra_info),
         log_line_(NULL),
         stack_copy_(NULL),
@@ -368,6 +370,11 @@ class MicrodumpWriter {
   }
 
   void DumpThreadStack() {
+    if (sanitize_stack_) {
+      dumper_->SanitizeStackCopy(stack_copy_, stack_len_, stack_pointer_,
+                                 stack_pointer_ - stack_lower_bound_);
+    }
+
     LogAppend("S 0 ");
     LogAppend(stack_pointer_);
     LogAppend(" ");
@@ -580,6 +587,7 @@ class MicrodumpWriter {
   const MappingList& mapping_list_;
   bool skip_dump_if_principal_mapping_not_referenced_;
   uintptr_t address_within_principal_mapping_;
+  bool sanitize_stack_;
   const MicrodumpExtraInfo microdump_extra_info_;
   char* log_line_;
 
@@ -607,6 +615,7 @@ bool WriteMicrodump(pid_t crashing_process,
                     const MappingList& mappings,
                     bool skip_dump_if_principal_mapping_not_referenced,
                     uintptr_t address_within_principal_mapping,
+                    bool sanitize_stack,
                     const MicrodumpExtraInfo& microdump_extra_info) {
   LinuxPtraceDumper dumper(crashing_process);
   const ExceptionHandler::CrashContext* context = NULL;
@@ -619,9 +628,10 @@ bool WriteMicrodump(pid_t crashing_process,
     dumper.set_crash_signal(context->siginfo.si_signo);
     dumper.set_crash_thread(context->tid);
   }
-  MicrodumpWriter writer(
-      context, mappings, skip_dump_if_principal_mapping_not_referenced,
-      address_within_principal_mapping, microdump_extra_info, &dumper);
+  MicrodumpWriter writer(context, mappings,
+                         skip_dump_if_principal_mapping_not_referenced,
+                         address_within_principal_mapping, sanitize_stack,
+                         microdump_extra_info, &dumper);
   if (!writer.Init())
     return false;
   writer.Dump();
diff --git a/src/client/linux/microdump_writer/microdump_writer.h b/src/client/linux/microdump_writer/microdump_writer.h
index 78af86df..a1e53df6 100644
--- a/src/client/linux/microdump_writer/microdump_writer.h
+++ b/src/client/linux/microdump_writer/microdump_writer.h
@@ -60,6 +60,7 @@ bool WriteMicrodump(pid_t crashing_process,
                     const MappingList& mappings,
                     bool skip_dump_if_main_module_not_referenced,
                     uintptr_t address_within_main_module,
+                    bool sanitize_stack,
                     const MicrodumpExtraInfo& microdump_extra_info);
 
 }  // namespace google_breakpad
diff --git a/src/client/linux/microdump_writer/microdump_writer_unittest.cc b/src/client/linux/microdump_writer/microdump_writer_unittest.cc
index 5d028bd3..e5e6f448 100644
--- a/src/client/linux/microdump_writer/microdump_writer_unittest.cc
+++ b/src/client/linux/microdump_writer/microdump_writer_unittest.cc
@@ -73,11 +73,14 @@ bool ContainsMicrodump(const std::string& buf) {
          std::string::npos != buf.find("-----END BREAKPAD MICRODUMP-----");
 }
 
+const char kIdentifiableString[] = "_IDENTIFIABLE_";
+
 void CrashAndGetMicrodump(const MappingList& mappings,
                           const MicrodumpExtraInfo& microdump_extra_info,
                           std::string* microdump,
                           bool skip_dump_if_principal_mapping_not_referenced = false,
-                          uintptr_t address_within_principal_mapping = 0) {
+                          uintptr_t address_within_principal_mapping = 0,
+                          bool sanitize_stack = false) {
   int fds[2];
   ASSERT_NE(-1, pipe(fds));
 
@@ -86,6 +89,14 @@ void CrashAndGetMicrodump(const MappingList& mappings,
   int err_fd = open(stderr_file.c_str(), O_CREAT | O_RDWR, S_IRUSR | S_IWUSR);
   ASSERT_NE(-1, err_fd);
 
+  char identifiable_string[sizeof(kIdentifiableString)];
+
+  // This string should not appear in the resulting microdump if it
+  // has been sanitized.
+  strcpy(identifiable_string, kIdentifiableString);
+  // Force the strcpy to not be optimized away.
+  write(STDOUT_FILENO, identifiable_string, 0);
+
   const pid_t child = fork();
   if (child == 0) {
     close(fds[1]);
@@ -112,7 +123,8 @@ void CrashAndGetMicrodump(const MappingList& mappings,
 
   ASSERT_TRUE(WriteMicrodump(child, &context, sizeof(context), mappings,
                              skip_dump_if_principal_mapping_not_referenced,
-                             address_within_principal_mapping, microdump_extra_info));
+                             address_within_principal_mapping, sanitize_stack,
+                             microdump_extra_info));
 
   // Revert stderr back to the console.
   dup2(save_err, STDERR_FILENO);
@@ -134,6 +146,27 @@ void CrashAndGetMicrodump(const MappingList& mappings,
   close(fds[1]);
 }
 
+void ExtractMicrodumpStackContents(const string& microdump_content,
+                                   string* result) {
+  std::istringstream iss(microdump_content);
+  result->clear();
+  for (string line; std::getline(iss, line);) {
+    if (line.find("S ") == 0) {
+      std::istringstream stack_data(line);
+      std::string key;
+      std::string addr;
+      std::string data;
+      stack_data >> key >> addr >> data;
+      EXPECT_TRUE((data.size() & 1u) == 0u);
+      result->reserve(result->size() + data.size() / 2);
+      for (size_t i = 0; i < data.size(); i += 2) {
+        std::string byte = data.substr(i, 2);
+        result->push_back(static_cast<char>(strtoul(byte.c_str(), NULL, 16)));
+      }
+    }
+  }
+}
+
 void CheckMicrodumpContents(const string& microdump_content,
                             const MicrodumpExtraInfo& expected_info) {
   std::istringstream iss(microdump_content);
@@ -175,6 +208,13 @@ void CheckMicrodumpContents(const string& microdump_content,
   ASSERT_TRUE(did_find_gpu_info);
 }
 
+bool MicrodumpStackContains(const string& microdump_content,
+                            const string& expected_content) {
+  string result;
+  ExtractMicrodumpStackContents(microdump_content, &result);
+  return result.find(kIdentifiableString) != string::npos;
+}
+
 void CheckMicrodumpContents(const string& microdump_content,
                             const string& expected_fingerprint,
                             const string& expected_product_info,
@@ -244,6 +284,46 @@ TEST(MicrodumpWriterTest, NoOutputIfUninteresting) {
   ASSERT_FALSE(ContainsMicrodump(buf));
 }
 
+// Ensure that stack content does not contain an identifiable string if the
+// stack is sanitized.
+TEST(MicrodumpWriterTest, StringRemovedBySanitization) {
+  const char kProductInfo[] = "MockProduct:42.0.2311.99";
+  const char kBuildFingerprint[] =
+      "aosp/occam/mako:5.1.1/LMY47W/12345678:userdegbug/dev-keys";
+  const char kGPUFingerprint[] =
+      "Qualcomm;Adreno (TM) 330;OpenGL ES 3.0 V@104.0 AU@  (GIT@Id3510ff6dc)";
+
+  const MicrodumpExtraInfo kMicrodumpExtraInfo(
+      MakeMicrodumpExtraInfo(kBuildFingerprint, kProductInfo, kGPUFingerprint));
+
+  std::string buf;
+  MappingList no_mappings;
+
+  CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfo, &buf, false, 0u, true);
+  ASSERT_TRUE(ContainsMicrodump(buf));
+  ASSERT_FALSE(MicrodumpStackContains(buf, kIdentifiableString));
+}
+
+// Ensure that stack content does contain an identifiable string if the
+// stack is not sanitized.
+TEST(MicrodumpWriterTest, StringPresentIfNotSanitized) {
+  const char kProductInfo[] = "MockProduct:42.0.2311.99";
+  const char kBuildFingerprint[] =
+      "aosp/occam/mako:5.1.1/LMY47W/12345678:userdegbug/dev-keys";
+  const char kGPUFingerprint[] =
+      "Qualcomm;Adreno (TM) 330;OpenGL ES 3.0 V@104.0 AU@  (GIT@Id3510ff6dc)";
+
+  const MicrodumpExtraInfo kMicrodumpExtraInfo(
+      MakeMicrodumpExtraInfo(kBuildFingerprint, kProductInfo, kGPUFingerprint));
+
+  std::string buf;
+  MappingList no_mappings;
+
+  CrashAndGetMicrodump(no_mappings, kMicrodumpExtraInfo, &buf, false, 0u, false);
+  ASSERT_TRUE(ContainsMicrodump(buf));
+  ASSERT_TRUE(MicrodumpStackContains(buf, kIdentifiableString));
+}
+
 // Ensure that output occurs if the interest region is set, and
 // does overlap something on the stack.
 TEST(MicrodumpWriterTest, OutputIfInteresting) {