Skip to content

Commit

Permalink
Fixing integer overflows in Breakpad Processor map/range code.
Browse files Browse the repository at this point in the history
Recently, Breakpad symbol files have exceeded the various 32-bit limits in these utils and we started seeing integer overflows.

This is also fixing a build issue in src/common/mac/dump_syms.cc.

Change-Id: Ibd913816c3b2b1171ac9991718c8911ac31eda86
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/5299472
Reviewed-by: Ivan Penkov <[email protected]>
Reviewed-by: Joshua Peraza <[email protected]>
  • Loading branch information
Ivan Penkov committed Feb 16, 2024
1 parent 55036dd commit f80f288
Show file tree
Hide file tree
Showing 20 changed files with 178 additions and 172 deletions.
4 changes: 2 additions & 2 deletions src/common/mac/dump_syms.cc
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ bool DumpSymbols::CreateEmptyModule(scoped_ptr<Module>& module) {
selected_object_name_ = object_filename_;
if (object_files_.size() > 1) {
selected_object_name_ += ", architecture ";
selected_object_name_ + selected_arch_name;
selected_object_name_ += selected_arch_name;
}

// Compute a module name, to appear in the MODULE record.
Expand Down Expand Up @@ -537,7 +537,7 @@ void DumpSymbols::ReadDwarf(google_breakpad::Module* module,
selected_object_name_, offset);
}
DwarfCUToModule root_handler(&file_context, &line_to_module,
&ranges_handler, reporter,
&ranges_handler, reporter.get(),
handle_inline);
// Make a Dwarf2Handler that drives our DIEHandler.
DIEDispatcher die_dispatcher(&root_handler);
Expand Down
2 changes: 1 addition & 1 deletion src/processor/fast_source_line_resolver_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ struct FastSourceLineResolver::Inline : public SourceLineResolverBase::Inline {

// De-serialize the memory data of a Inline.
void CopyFrom(const char* raw) {
DESERIALIZE(raw, has_call_site_file_id);
raw = SimpleSerializer<bool>::Read(raw, &has_call_site_file_id);
DESERIALIZE(raw, inline_nest_level);
DESERIALIZE(raw, call_site_line);
DESERIALIZE(raw, call_site_file_id);
Expand Down
11 changes: 8 additions & 3 deletions src/processor/logging.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@
#define PROCESSOR_LOGGING_H__

#include <iostream>
#include <sstream>
#include <string>
#include <iomanip>

#include "common/using_std_string.h"
#include "google_breakpad/common/breakpad_types.h"
Expand Down Expand Up @@ -119,9 +121,12 @@ class LogMessageVoidify {
};

// Returns number formatted as a hexadecimal string, such as "0x7b".
string HexString(uint32_t number);
string HexString(uint64_t number);
string HexString(int number);
template<typename T>
string HexString(T number) {
std::stringstream stream;
stream << "0x" << std::hex << number;
return stream.str();
}

// Returns the error code as set in the global errno variable, and sets
// error_string, a required argument, to a string describing that error
Expand Down
56 changes: 28 additions & 28 deletions src/processor/map_serializers-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ template<typename Key, typename Value>
size_t StdMapSerializer<Key, Value>::SizeOf(
const std::map<Key, Value>& m) const {
size_t size = 0;
size_t header_size = (1 + m.size()) * sizeof(uint32_t);
size_t header_size = (1 + m.size()) * sizeof(uint64_t);
size += header_size;

typename std::map<Key, Value>::const_iterator iter;
Expand All @@ -76,19 +76,19 @@ char* StdMapSerializer<Key, Value>::Write(const std::map<Key, Value>& m,

// Write header:
// Number of nodes.
dest = SimpleSerializer<uint32_t>::Write(m.size(), dest);
dest = SimpleSerializer<uint64_t>::Write(m.size(), dest);
// Nodes offsets.
uint32_t* offsets = reinterpret_cast<uint32_t*>(dest);
dest += sizeof(uint32_t) * m.size();
uint64_t* offsets = reinterpret_cast<uint64_t*>(dest);
dest += sizeof(uint64_t) * m.size();

char* key_address = dest;
dest += sizeof(Key) * m.size();

// Traverse map.
typename std::map<Key, Value>::const_iterator iter;
int index = 0;
int64_t index = 0;
for (iter = m.begin(); iter != m.end(); ++iter, ++index) {
offsets[index] = static_cast<uint32_t>(dest - start_address);
offsets[index] = static_cast<uint64_t>(dest - start_address);
key_address = key_serializer_.Write(iter->first, key_address);
dest = value_serializer_.Write(iter->second, dest);
}
Expand All @@ -97,9 +97,9 @@ char* StdMapSerializer<Key, Value>::Write(const std::map<Key, Value>& m,

template<typename Key, typename Value>
char* StdMapSerializer<Key, Value>::Serialize(
const std::map<Key, Value>& m, unsigned int* size) const {
const std::map<Key, Value>& m, uint64_t* size) const {
// Compute size of memory to be allocated.
unsigned int size_to_alloc = SizeOf(m);
uint64_t size_to_alloc = SizeOf(m);
// Allocate memory.
char* serialized_data = new char[size_to_alloc];
if (!serialized_data) {
Expand All @@ -118,7 +118,7 @@ template<typename Address, typename Entry>
size_t RangeMapSerializer<Address, Entry>::SizeOf(
const RangeMap<Address, Entry>& m) const {
size_t size = 0;
size_t header_size = (1 + m.map_.size()) * sizeof(uint32_t);
size_t header_size = (1 + m.map_.size()) * sizeof(uint64_t);
size += header_size;

typename std::map<Address, Range>::const_iterator iter;
Expand All @@ -144,19 +144,19 @@ char* RangeMapSerializer<Address, Entry>::Write(

// Write header:
// Number of nodes.
dest = SimpleSerializer<uint32_t>::Write(m.map_.size(), dest);
dest = SimpleSerializer<uint64_t>::Write(m.map_.size(), dest);
// Nodes offsets.
uint32_t* offsets = reinterpret_cast<uint32_t*>(dest);
dest += sizeof(uint32_t) * m.map_.size();
uint64_t* offsets = reinterpret_cast<uint64_t*>(dest);
dest += sizeof(uint64_t) * m.map_.size();

char* key_address = dest;
dest += sizeof(Address) * m.map_.size();

// Traverse map.
typename std::map<Address, Range>::const_iterator iter;
int index = 0;
int64_t index = 0;
for (iter = m.map_.begin(); iter != m.map_.end(); ++iter, ++index) {
offsets[index] = static_cast<uint32_t>(dest - start_address);
offsets[index] = static_cast<uint64_t>(dest - start_address);
key_address = address_serializer_.Write(iter->first, key_address);
dest = address_serializer_.Write(iter->second.base(), dest);
dest = entry_serializer_.Write(iter->second.entry(), dest);
Expand All @@ -166,9 +166,9 @@ char* RangeMapSerializer<Address, Entry>::Write(

template<typename Address, typename Entry>
char* RangeMapSerializer<Address, Entry>::Serialize(
const RangeMap<Address, Entry>& m, unsigned int* size) const {
const RangeMap<Address, Entry>& m, uint64_t* size) const {
// Compute size of memory to be allocated.
unsigned int size_to_alloc = SizeOf(m);
uint64_t size_to_alloc = SizeOf(m);
// Allocate memory.
char* serialized_data = new char[size_to_alloc];
if (!serialized_data) {
Expand All @@ -191,12 +191,12 @@ size_t ContainedRangeMapSerializer<AddrType, EntryType>::SizeOf(
size_t size = 0;
size_t header_size = addr_serializer_.SizeOf(m->base_)
+ entry_serializer_.SizeOf(m->entry_)
+ sizeof(uint32_t);
+ sizeof(uint64_t);
size += header_size;
// In case m.map_ == NULL, we treat it as an empty map:
size += sizeof(uint32_t);
size += sizeof(uint64_t);
if (m->map_) {
size += m->map_->size() * sizeof(uint32_t);
size += m->map_->size() * sizeof(uint64_t);
typename Map::const_iterator iter;
for (iter = m->map_->begin(); iter != m->map_->end(); ++iter) {
size += addr_serializer_.SizeOf(iter->first);
Expand All @@ -215,27 +215,27 @@ char* ContainedRangeMapSerializer<AddrType, EntryType>::Write(
return NULL;
}
dest = addr_serializer_.Write(m->base_, dest);
dest = SimpleSerializer<uint32_t>::Write(entry_serializer_.SizeOf(m->entry_),
dest = SimpleSerializer<uint64_t>::Write(entry_serializer_.SizeOf(m->entry_),
dest);
dest = entry_serializer_.Write(m->entry_, dest);

// Write map<<AddrType, ContainedRangeMap*>:
char* map_address = dest;
if (m->map_ == NULL) {
dest = SimpleSerializer<uint32_t>::Write(0, dest);
dest = SimpleSerializer<uint64_t>::Write(0, dest);
} else {
dest = SimpleSerializer<uint32_t>::Write(m->map_->size(), dest);
uint32_t* offsets = reinterpret_cast<uint32_t*>(dest);
dest += sizeof(uint32_t) * m->map_->size();
dest = SimpleSerializer<uint64_t>::Write(m->map_->size(), dest);
uint64_t* offsets = reinterpret_cast<uint64_t*>(dest);
dest += sizeof(uint64_t) * m->map_->size();

char* key_address = dest;
dest += sizeof(AddrType) * m->map_->size();

// Traverse map.
typename Map::const_iterator iter;
int index = 0;
int64_t index = 0;
for (iter = m->map_->begin(); iter != m->map_->end(); ++iter, ++index) {
offsets[index] = static_cast<uint32_t>(dest - map_address);
offsets[index] = static_cast<uint64_t>(dest - map_address);
key_address = addr_serializer_.Write(iter->first, key_address);
// Recursively write.
dest = Write(iter->second, dest);
Expand All @@ -246,8 +246,8 @@ char* ContainedRangeMapSerializer<AddrType, EntryType>::Write(

template<class AddrType, class EntryType>
char* ContainedRangeMapSerializer<AddrType, EntryType>::Serialize(
const ContainedRangeMap<AddrType, EntryType>* m, unsigned int* size) const {
unsigned int size_to_alloc = SizeOf(m);
const ContainedRangeMap<AddrType, EntryType>* m, uint64_t* size) const {
uint64_t size_to_alloc = SizeOf(m);
// Allocating memory.
char* serialized_data = new char[size_to_alloc];
if (!serialized_data) {
Expand Down
8 changes: 4 additions & 4 deletions src/processor/map_serializers.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class StdMapSerializer {
// Returns a pointer to the serialized data. If size != NULL, *size is set
// to the size of serialized data, i.e., SizeOf(m).
// Caller has the ownership of memory allocated as "new char[]".
char* Serialize(const std::map<Key, Value>& m, unsigned int* size) const;
char* Serialize(const std::map<Key, Value>& m, uint64_t* size) const;

private:
SimpleSerializer<Key> key_serializer_;
Expand Down Expand Up @@ -93,7 +93,7 @@ class AddressMapSerializer {
// Returns a pointer to the serialized data. If size != NULL, *size is set
// to the size of serialized data, i.e., SizeOf(m).
// Caller has the ownership of memory allocated as "new char[]".
char* Serialize(const AddressMap<Addr, Entry>& m, unsigned int* size) const {
char* Serialize(const AddressMap<Addr, Entry>& m, uint64_t* size) const {
return std_map_serializer_.Serialize(m.map_, size);
}

Expand All @@ -120,7 +120,7 @@ class RangeMapSerializer {
// Returns a pointer to the serialized data. If size != NULL, *size is set
// to the size of serialized data, i.e., SizeOf(m).
// Caller has the ownership of memory allocated as "new char[]".
char* Serialize(const RangeMap<Address, Entry>& m, unsigned int* size) const;
char* Serialize(const RangeMap<Address, Entry>& m, uint64_t* size) const;

private:
// Convenient type name for Range.
Expand Down Expand Up @@ -151,7 +151,7 @@ class ContainedRangeMapSerializer {
// to the size of serialized data, i.e., SizeOf(m).
// Caller has the ownership of memory allocated as "new char[]".
char* Serialize(const ContainedRangeMap<AddrType, EntryType>* m,
unsigned int* size) const;
uint64_t* size) const;

private:
// Convenient type name for the underlying map type.
Expand Down
Loading

0 comments on commit f80f288

Please sign in to comment.