Robust ELF header parsing.

courgette/disassembler_elf_32.cc

Index: courgette/disassembler_elf_32.cc
diff --git a/courgette/disassembler_elf_32.cc b/courgette/disassembler_elf_32.cc
index 02a31eb6dd734f2353bb228c7847d2d8592e95e0..ec7cef541f9fff172ee467b09347476a85d7a632 100644
--- a/courgette/disassembler_elf_32.cc
+++ b/courgette/disassembler_elf_32.cc
@@ -54,27 +54,26 @@ bool DisassemblerElf32::ParseHeader() {
   if (header_->e_shentsize != sizeof(Elf32_Shdr))
     return Bad("Unexpected section header size");
 
-  if (header_->e_shoff >= length())
-    return Bad("Out of bounds section header table offset");
+  if (!IsArrayInBounds(header_->e_shoff, header_->e_shnum, sizeof(Elf32_Shdr)))
+    return Bad("Out of bounds section header table");
 
   section_header_table_ = (Elf32_Shdr *)OffsetToPointer(header_->e_shoff);
   section_header_table_size_ = header_->e_shnum;
 
-  if ((header_->e_shoff + header_->e_shnum ) >= length())
-    return Bad("Out of bounds section header table");
-
-  if (header_->e_phoff >= length())
-    return Bad("Out of bounds program header table offset");
+  if (!IsArrayInBounds(header_->e_phoff, header_->e_phnum, sizeof(Elf32_Phdr)))
+    return Bad("Out of bounds program header table");
 
   program_header_table_ = (Elf32_Phdr *)OffsetToPointer(header_->e_phoff);
   program_header_table_size_ = header_->e_phnum;
 
-  if ((header_->e_phoff + header_->e_phnum) >= length())
-    return Bad("Out of bounds program header table");
+  if (header_->e_shstrndx >= header_->e_shnum)
+    return Bad("Out of bounds string section index");
 
   default_string_section_ = (const char *)SectionBody((int)header_->e_shstrndx);
 
-  ReduceLength(DiscoverLength());
+  if (!UpdateLength()) {
+    return Bad("Out of bounds section or segment");
+  }
 
   return Good();
 }
@@ -100,7 +99,7 @@ bool DisassemblerElf32::Disassemble(AssemblyProgram* target) {
   return true;
 }
 
-uint32 DisassemblerElf32::DiscoverLength() {
+bool DisassemblerElf32::UpdateLength() {
   uint32 result = 0;

[rickyz (no longer on Chrome), 2015/03/23 23:14:50]

Mind changing this to an Elf32_Off as well?

[halyavin2, 2015/03/24 14:35:18]

Done. 

I just realized there is one more bug to fix though. If length() is more than
4Gb, we may not notice 32-bit integer overflow. I filed crbug.com/470109.

 
   // Find the end of the last section
@@ -110,33 +109,34 @@ uint32 DisassemblerElf32::DiscoverLength() {
     if (section_header->sh_type == SHT_NOBITS)
       continue;
 
-    uint32 section_end = section_header->sh_offset + section_header->sh_size;
+    if (!IsArrayInBounds(section_header->sh_offset, section_header->sh_size, 1))
+      return false;
 
-    if (section_end > result)
-      result = section_end;
+    uint32 section_end = section_header->sh_offset + section_header->sh_size;
+    result = std::max(result, section_end);
   }
 
   // Find the end of the last segment
   for (int i = 0; i < ProgramSegmentHeaderCount(); i++) {
     const Elf32_Phdr *segment_header = ProgramSegmentHeader(i);
 
-    uint32 segment_end = segment_header->p_offset + segment_header->p_filesz;
+    if (!IsArrayInBounds(segment_header->p_offset, segment_header->p_filesz, 1))
+      return false;
 
-    if (segment_end > result)
-      result = segment_end;
+    uint32 segment_end = segment_header->p_offset + segment_header->p_filesz;
+    result = std::max(result, segment_end);
   }
 
   uint32 section_table_end = header_->e_shoff +
                              (header_->e_shnum * sizeof(Elf32_Shdr));
-  if (section_table_end > result)
-    result = section_table_end;
+  result = std::max(result, section_table_end);
 
   uint32 segment_table_end = header_->e_phoff +
                              (header_->e_phnum * sizeof(Elf32_Phdr));
-  if (segment_table_end > result)
-    result = segment_table_end;
+  result = std::max(result, segment_table_end);
 
-  return result;
+  ReduceLength(result);
+  return true;
 }
 
 CheckBool DisassemblerElf32::IsValidRVA(RVA rva) const {