Skip to content

Commit

Permalink
[Courgette] Fix Fuzzer failure for ARM reloc parsing.
Browse files Browse the repository at this point in the history
The Courgette Fuzzer found a bug where
DisassemblerElf32ARM::ParseRelocationSection() overruns buffer
because reloc units are not sorted. This CL fixes the bug by adding
a check. The bug does not happen for X86 case, or for Zucchini.

Bug: 961540
Change-Id: I5ff49c510e9fd8083f42a34e99140ff2c47201e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1818875
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698927}
  • Loading branch information
samuelhuang authored and Commit Bot committed Sep 23, 2019
1 parent 70d1c6f commit e791cc7
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
18 changes: 14 additions & 4 deletions courgette/disassembler_elf_32_arm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -365,12 +365,16 @@ CheckBool DisassemblerElf32ARM::ParseRelocationSection(
if (abs32_locations_.size() > section_relocs_count)
match = false;

if (!abs32_locations_.empty()) {
if (match && !abs32_locations_.empty()) {
std::vector<RVA>::const_iterator reloc_iter = abs32_locations_.begin();

for (uint32_t i = 0; i < section_relocs_count; ++i) {
if (section_relocs_iter->r_offset == *reloc_iter)
// Look for the first reloc unit matching |abs32_locations_[0]|.
size_t section_relocs_remaining = section_relocs_count;
for (; section_relocs_remaining > 0; --section_relocs_remaining) {
if (section_relocs_iter->r_info == R_ARM_RELATIVE &&
section_relocs_iter->r_offset == *reloc_iter) {
break;
}

if (!ParseSimpleRegion(file_offset, file_offset + sizeof(Elf32_Rel),
receptor)) {
Expand All @@ -381,6 +385,12 @@ CheckBool DisassemblerElf32ARM::ParseRelocationSection(
++section_relocs_iter;
}

// If there aren't enough reloc units left then don't bother matching. This
// can happen if the reloc units are not sorted.
if (abs32_locations_.size() > section_relocs_remaining)
match = false;

// Try to match successive reloc units with (sorted) |abs32_locations_|.
while (match && (reloc_iter != abs32_locations_.end())) {
if (section_relocs_iter->r_info != R_ARM_RELATIVE ||
section_relocs_iter->r_offset != *reloc_iter) {
Expand All @@ -393,7 +403,7 @@ CheckBool DisassemblerElf32ARM::ParseRelocationSection(
}

if (match) {
// Skip over relocation tables
// Success: Emit relocation table.
if (!receptor->EmitElfARMRelocation())
return false;
}
Expand Down
3 changes: 2 additions & 1 deletion courgette/disassembler_elf_32_x86.cc
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ CheckBool DisassemblerElf32X86::ParseRelocationSection(

std::vector<RVA>::const_iterator reloc_iter = abs32_locations_.begin();

// Try to match successive reloc units with (sorted) |abs32_locations_|.
while (match && (reloc_iter != abs32_locations_.end())) {
if (section_relocs_iter->r_info != R_386_RELATIVE ||
section_relocs_iter->r_offset != *reloc_iter) {
Expand All @@ -118,7 +119,7 @@ CheckBool DisassemblerElf32X86::ParseRelocationSection(
}

if (match) {
// Skip over relocation tables.
// Success: Emit relocation table.
if (!receptor->EmitElfRelocation())
return false;
file_offset += sizeof(Elf32_Rel) * abs32_locations_.size();
Expand Down

0 comments on commit e791cc7

Please sign in to comment.