Fix unchecked allocation size of in fuzzer note reading

This fixes a fuzzer-only bug, and modifies the note API so that it can
no longer request infinitely sized notes.

Bug: chromium:966303
Change-Id: I97b9ca6774d3101560caddf2f9b0a8d7ecf7c2e2
Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/1628675
Commit-Queue: Scott Graham <scottmg@chromium.org>
Reviewed-by: Joshua Peraza <jperaza@chromium.org>
This commit is contained in:
Scott Graham 2019-05-24 12:49:28 -07:00 committed by Commit Bot
parent 122363ccae
commit daf9f5669e
4 changed files with 26 additions and 17 deletions

View File

@ -228,10 +228,19 @@ ElfImageReader::NoteReader::Result ElfImageReader::NoteReader::NextNote(
return Result::kError;
}
namespace {
// The maximum size the user can specify for maximum note size. Clamping this
// ensures that buffer allocations cannot be wildly large. It is not expected
// that a note would be larger than ~1k in normal usage.
constexpr size_t kMaxMaxNoteSize = 16384;
} // namespace
ElfImageReader::NoteReader::NoteReader(const ElfImageReader* elf_reader,
const ProcessMemoryRange* range,
const ProgramHeaderTable* phdr_table,
ssize_t max_note_size,
size_t max_note_size,
const std::string& name_filter,
NoteType type_filter,
bool use_filter)
@ -242,12 +251,14 @@ ElfImageReader::NoteReader::NoteReader(const ElfImageReader* elf_reader,
phdr_table_(phdr_table),
segment_range_(),
phdr_index_(0),
max_note_size_(max_note_size),
max_note_size_(std::min(kMaxMaxNoteSize, max_note_size)),
name_filter_(name_filter),
type_filter_(type_filter),
use_filter_(use_filter),
is_valid_(true),
retry_(false) {}
retry_(false) {
DCHECK_LT(max_note_size, kMaxMaxNoteSize);
}
template <typename NhdrType>
ElfImageReader::NoteReader::Result ElfImageReader::NoteReader::ReadNote(
@ -282,7 +293,7 @@ ElfImageReader::NoteReader::Result ElfImageReader::NoteReader::ReadNote(
std::min(PAD(current_address_ + note_size), segment_end_address_);
#undef PAD
if (max_note_size_ >= 0 && note_size > static_cast<size_t>(max_note_size_)) {
if (note_size > max_note_size_) {
current_address_ = end_of_note;
retry_ = true;
return Result::kError;
@ -790,7 +801,7 @@ bool ElfImageReader::GetNumberOfSymbolEntriesFromDtGnuHash(
}
std::unique_ptr<ElfImageReader::NoteReader> ElfImageReader::Notes(
ssize_t max_note_size) {
size_t max_note_size) {
return std::make_unique<NoteReader>(
this, &memory_, program_headers_.get(), max_note_size);
}
@ -798,7 +809,7 @@ std::unique_ptr<ElfImageReader::NoteReader> ElfImageReader::Notes(
std::unique_ptr<ElfImageReader::NoteReader>
ElfImageReader::NotesWithNameAndType(const std::string& name,
NoteReader::NoteType type,
ssize_t max_note_size) {
size_t max_note_size) {
return std::make_unique<NoteReader>(
this, &memory_, program_headers_.get(), max_note_size, name, type, true);
}

View File

@ -83,7 +83,7 @@ class ElfImageReader {
NoteReader(const ElfImageReader* elf_reader_,
const ProcessMemoryRange* range,
const ProgramHeaderTable* phdr_table,
ssize_t max_note_size,
size_t max_note_size,
const std::string& name_filter = std::string(),
NoteType type_filter = 0,
bool use_filter = false);
@ -105,7 +105,7 @@ class ElfImageReader {
const ProgramHeaderTable* phdr_table_; // weak
std::unique_ptr<ProcessMemoryRange> segment_range_;
size_t phdr_index_;
ssize_t max_note_size_;
size_t max_note_size_;
std::string name_filter_;
NoteType type_filter_;
bool use_filter_;
@ -211,10 +211,9 @@ class ElfImageReader {
//!
//! \param[in] max_note_size The maximum note size to read. Notes whose
//! combined name, descriptor, and padding size are greater than
//! \a max_note_size will be silently skipped. A \a max_note_size of -1
//! indicates infinite maximum note size.
//! \a max_note_size will be silently skipped.
//! \return A NoteReader object capable of reading notes in this image.
std::unique_ptr<NoteReader> Notes(ssize_t max_note_size);
std::unique_ptr<NoteReader> Notes(size_t max_note_size);
//! \brief Return a NoteReader for this image, which scans all PT_NOTE
//! segments in the image, filtering by name and type.
@ -226,12 +225,11 @@ class ElfImageReader {
//! \param[in] type The note type to match.
//! \param[in] max_note_size The maximum note size to read. Notes whose
//! combined name, descriptor, and padding size are greater than
//! \a max_note_size will be silently skipped. A \a max_note_size of -1
//! indicates infinite maximum note size.
//! \a max_note_size will be silently skipped.
//! \return A NoteReader object capable of reading notes in this image.
std::unique_ptr<NoteReader> NotesWithNameAndType(const std::string& name,
NoteReader::NoteType type,
ssize_t max_note_size);
size_t max_note_size);
//! \brief Return a ProcessMemoryRange restricted to the range of this image.
//!

View File

@ -67,7 +67,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
std::string note_desc;
ElfImageReader::NoteReader::NoteType note_type;
VMAddress desc_addr;
auto notes = reader.Notes(-1);
auto notes = reader.Notes(9999);
while ((result = notes->NextNote(
&note_name, &note_type, &note_desc, &desc_addr)) ==
ElfImageReader::NoteReader::Result::kSuccess) {

View File

@ -155,7 +155,7 @@ void ReadThisExecutableInTarget(ProcessType process,
ElfImageReader::NoteReader::NoteType note_type;
VMAddress desc_addr;
std::unique_ptr<ElfImageReader::NoteReader> notes = reader.Notes(-1);
std::unique_ptr<ElfImageReader::NoteReader> notes = reader.Notes(10000);
while ((result = notes->NextNote(
&note_name, &note_type, &note_desc, &desc_addr)) ==
ElfImageReader::NoteReader::Result::kSuccess) {
@ -169,7 +169,7 @@ void ReadThisExecutableInTarget(ProcessType process,
// Find the note defined in elf_image_reader_test_note.S.
constexpr uint32_t kCrashpadNoteDesc = 42;
notes = reader.NotesWithNameAndType(
CRASHPAD_ELF_NOTE_NAME, CRASHPAD_ELF_NOTE_TYPE_SNAPSHOT_TEST, -1);
CRASHPAD_ELF_NOTE_NAME, CRASHPAD_ELF_NOTE_TYPE_SNAPSHOT_TEST, 10000);
ASSERT_EQ(notes->NextNote(&note_name, &note_type, &note_desc, &desc_addr),
ElfImageReader::NoteReader::Result::kSuccess);
EXPECT_EQ(note_name, CRASHPAD_ELF_NOTE_NAME);