Fixing a buffer overflow in the Breakpad symbol serialization code caused by an integer overflow.

We started getting Breakpad symbol files that exceed some of the 32-bit limits.

Change-Id: I1f81905c0b7f88a0c93f10b651e5e160876487fd
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/5186360
Reviewed-by: Joshua Peraza <jperaza@chromium.org>

src/processor/fast_source_line_resolver.cc

@@ -44,6 +44,7 @@
 #include "processor/fast_source_line_resolver_types.h"
 
 #include <cassert>
+#include <cstdint>
 #include <map>
 #include <string>
 #include <utility>
@@ -154,7 +155,7 @@ void FastSourceLineResolver::Module::ConstructInlineFrames(
       }
     }
 
-    // Use the starting adress of the inlined range as inlined function base.
+    // Use the starting address of the inlined range as inlined function base.
     new_frame->function_base = new_frame->module->base_address();
     for (const auto& range : in->inline_ranges) {
       if (address >= range.first && address < range.first + range.second) {
@@ -228,21 +229,21 @@ bool FastSourceLineResolver::Module::LoadMapFromMemory(
   const char* mem_buffer = memory_buffer;
   mem_buffer = SimpleSerializer<bool>::Read(mem_buffer, &is_corrupt_);
 
-  const uint32_t* map_sizes = reinterpret_cast<const uint32_t*>(mem_buffer);
+  const uint64_t* map_sizes = reinterpret_cast<const uint64_t*>(mem_buffer);
 
-  unsigned int header_size = kNumberMaps_ * sizeof(unsigned int);
+  unsigned int header_size = kNumberMaps_ * sizeof(uint64_t);
 
   // offsets[]: an array of offset addresses (with respect to mem_buffer),
   // for each "Static***Map" component of Module.
   // "Static***Map": static version of std::map or map wrapper, i.e., StaticMap,
   // StaticAddressMap, StaticContainedRangeMap, and StaticRangeMap.
-  unsigned int offsets[kNumberMaps_];
+  uint64_t offsets[kNumberMaps_];
   offsets[0] = header_size;
   for (int i = 1; i < kNumberMaps_; ++i) {
     offsets[i] = offsets[i - 1] + map_sizes[i - 1];
   }
-  unsigned int expected_size = sizeof(bool) + offsets[kNumberMaps_ - 1] +
-                               map_sizes[kNumberMaps_ - 1] + 1;
+  size_t expected_size = sizeof(bool) + offsets[kNumberMaps_ - 1] +
+                         map_sizes[kNumberMaps_ - 1] + 1;
   if (expected_size != memory_buffer_size &&
       // Allow for having an extra null terminator.
       expected_size != memory_buffer_size - 1) {

src/processor/module_serializer.cc

@@ -38,11 +38,21 @@
 
 #include "processor/module_serializer.h"
 
+#include <cstdint>
+#include <cstring>
 #include <map>
 #include <string>
 
+#include "common/scoped_ptr.h"
+#include "google_breakpad/processor/basic_source_line_resolver.h"
+#include "google_breakpad/processor/code_module.h"
+#include "google_breakpad/processor/fast_source_line_resolver.h"
 #include "processor/basic_code_module.h"
+#include "processor/linked_ptr.h"
 #include "processor/logging.h"
+#include "processor/map_serializers.h"
+#include "processor/simple_serializer.h"
+#include "processor/windows_frame_info.h"
 
 namespace google_breakpad {
 
@@ -78,7 +88,7 @@ size_t ModuleSerializer::SizeOf(const BasicSourceLineResolver::Module& module) {
       inline_origin_serializer_.SizeOf(module.inline_origins_);
 
   // Header size.
-  total_size_alloc_ += kNumberMaps_ * sizeof(uint32_t);
+  total_size_alloc_ += kNumberMaps_ * sizeof(uint64_t);
 
   for (int i = 0; i < kNumberMaps_; ++i) {
     total_size_alloc_ += map_sizes_[i];
@@ -95,8 +105,8 @@ char* ModuleSerializer::Write(const BasicSourceLineResolver::Module& module,
   // Write the is_corrupt flag.
   dest = SimpleSerializer<bool>::Write(module.is_corrupt_, dest);
   // Write header.
-  memcpy(dest, map_sizes_, kNumberMaps_ * sizeof(uint32_t));
-  dest += kNumberMaps_ * sizeof(uint32_t);
+  memcpy(dest, map_sizes_, kNumberMaps_ * sizeof(uint64_t));
+  dest += kNumberMaps_ * sizeof(uint64_t);
   // Write each map.
   dest = files_serializer_.Write(module.files_, dest);
   dest = functions_serializer_.Write(module.functions_, dest);
@@ -161,7 +171,7 @@ bool ModuleSerializer::SerializeModuleAndLoadIntoFastResolver(
 
   // Copy the data into string.
   // Must pass string to LoadModuleUsingMapBuffer(), instead of passing char* to
-  // LoadModuleUsingMemoryBuffer(), becaused of data ownership/lifetime issue.
+  // LoadModuleUsingMemoryBuffer(), because of data ownership/lifetime issue.
   string symbol_data_string(symbol_data.get(), size);
   symbol_data.reset();
 
@@ -213,7 +223,7 @@ char* ModuleSerializer::SerializeSymbolFileData(const string& symbol_data,
     return NULL;
   }
   buffer.reset(NULL);
-  return Serialize(*(module.get()), size);
+  return Serialize(*module, size);
 }
 
 }  // namespace google_breakpad

src/processor/module_serializer.h

@@ -36,6 +36,7 @@
 #ifndef PROCESSOR_MODULE_SERIALIZER_H__
 #define PROCESSOR_MODULE_SERIALIZER_H__
 
+#include <cstdint>
 #include <map>
 #include <string>
 
@@ -110,7 +111,7 @@ class ModuleSerializer {
       FastSourceLineResolver::Module::kNumberMaps_;
 
   // Memory sizes required to serialize map components in Module.
-  uint32_t map_sizes_[kNumberMaps_];
+  uint64_t map_sizes_[kNumberMaps_];
 
   // Serializers for each individual map component in Module class.
   StdMapSerializer<int, string> files_serializer_;

src/processor/source_line_resolver_base.cc

@@ -253,7 +253,6 @@ bool SourceLineResolverBase::LoadModuleUsingMemoryBuffer(
     // Returning false from here would be an indication that the symbols for
     // this module are missing which would be wrong.  Intentionally fall through
     // and add the module to both the modules_ and the corrupt_modules_ lists.
-    assert(basic_module->IsCorrupt());
   }
 
   modules_->insert(make_pair(module->code_file(), basic_module));