From 7c2799f3ba6f8a8186c8883b213c3e59768b1287 Mon Sep 17 00:00:00 2001 From: Tobias Sargeant Date: Tue, 31 Jan 2017 13:42:52 +0000 Subject: 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 --- .../linux/microdump_writer/microdump_writer.cc | 16 ++++- .../linux/microdump_writer/microdump_writer.h | 1 + .../microdump_writer/microdump_writer_unittest.cc | 84 +++++++++++++++++++++- 3 files changed, 96 insertions(+), 5 deletions(-) (limited to 'src/client/linux/microdump_writer') 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(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) { -- cgit v1.2.1