diff --git a/cle/backends/coff.py b/cle/backends/coff.py index 556ca93b..cffa863a 100644 --- a/cle/backends/coff.py +++ b/cle/backends/coff.py @@ -11,6 +11,7 @@ import archinfo +from cle.errors import CLEInvalidBinaryError from cle.utils import extract_null_terminated_bytestr from .backend import Backend, register_backend @@ -173,6 +174,14 @@ def __init__(self, data: bytes): self.data: bytes = data self._parse() + def _require_in_file(self, offset: int, size: int, what: str) -> None: + # A table with no entries is never read, so nothing dereferences its pointer. + if size and offset + size > len(self.data): + raise CLEInvalidBinaryError( + f"The {what} needs {size:#x} bytes at {offset:#x}, " + f"which a {len(self.data):#x} byte file does not hold" + ) + def _parse(self) -> None: self.header = CoffFileHeader.from_buffer_copy(self.data) if self.header.Machine not in { @@ -181,8 +190,20 @@ def _parse(self) -> None: }: raise NotImplementedError("Unsupported machine type") - strings_offset = ( - self.header.PointerToSymbolTable + ctypes.sizeof(CoffSymbolTableEntry) * self.header.NumberOfSymbols + self._require_in_file( + ctypes.sizeof(self.header), + ctypes.sizeof(CoffSectionTableEntry) * self.header.NumberOfSections, + f"section table of {self.header.NumberOfSections} entries", + ) + + symbols_size = ctypes.sizeof(CoffSymbolTableEntry) * self.header.NumberOfSymbols + strings_offset = self.header.PointerToSymbolTable + symbols_size + # The string table begins on the byte after the last symbol and opens with its own size, + # so bounding that size field bounds every symbol table read as well. + self._require_in_file( + self.header.PointerToSymbolTable, + symbols_size + 4, + f"symbol table of {self.header.NumberOfSymbols} entries and the string table size after it", ) strings_size = struct.unpack(" None: # Relocations relocs = [] offset = section.PointerToRelocations + self._require_in_file( + offset, + ctypes.sizeof(CoffRelocationTableEntry) * section.NumberOfRelocations, + f"relocation table of section {i} with {section.NumberOfRelocations} entries", + ) for i in range(section.NumberOfRelocations): reloc = CoffRelocationTableEntry.from_buffer_copy(self.data, offset) relocs.append(reloc) @@ -413,7 +439,7 @@ def value(self): return offset_to_symbol -RELOC_CLASSES: dict[IntEnum, dict[IntEnum, type[Relocation]]] = { +RELOC_CLASSES: dict[IntEnum, dict[IntEnum, type[CoffRelocation]]] = { IMAGE_FILE_MACHINE.I386: { IMAGE_REL_I386.REL32: CoffRelocationREL32, IMAGE_REL_I386.DIR32: CoffRelocationDIR32, @@ -514,6 +540,21 @@ def _add_relocs(self) -> None: }: reloc_class = RELOC_CLASSES[self._coff.header.Machine].get(reloc.Type, None) if reloc_class is not None: + patch_size = struct.calcsize(reloc_class.PACK_FORMAT) + section_size = self._coff.sections[section_idx].SizeOfRawData + image_size = len(self._image_vmem) + if reloc.VirtualAddress + patch_size > section_size or patch_offset + patch_size > image_size: + log.warning( + "Section %s has a relocation of type %#x at %#x patching %#x bytes, which is out of " + "bounds for its SizeOfRawData %#x or for the image size %#x. Skipping this relocation.", + self._coff.get_section_name(section_idx), + reloc.Type, + reloc.VirtualAddress, + patch_size, + section_size, + image_size, + ) + continue cle_symbol = self.get_symbol(sym_name, produce_extern_symbols=True) self.relocs.append(reloc_class(self, cle_symbol, patch_offset)) continue diff --git a/tests/test_coff.py b/tests/test_coff.py index fcc30dd9..0b364969 100644 --- a/tests/test_coff.py +++ b/tests/test_coff.py @@ -37,6 +37,35 @@ def test_x86_64(self): assert "rejected" in symbol_names assert "authenticate" in symbol_names + def test_a_section_table_longer_than_the_file_is_rejected(self): + # The first 512 bytes of x86/fauxware.obj. Its header declares 29 sections, whose table + # needs 0x49c bytes counted from the start of the file. + exe = os.path.join(TEST_BASE, "tests", "x86", "coff_truncated_section_table.obj") + with self.assertRaisesRegex(cle.CLEInvalidBinaryError, "section table"): + cle.Loader(exe, auto_load_libs=False) + + def test_a_symbol_table_past_the_end_of_the_file_is_rejected(self): + # The first 2048 bytes of x86/fauxware.obj, which is long enough to hold the whole + # section table and not the 152 symbols at 0x31c1 or the string table after them. + exe = os.path.join(TEST_BASE, "tests", "x86", "coff_truncated_symbol_table.obj") + with self.assertRaisesRegex(cle.CLEInvalidBinaryError, "symbol table"): + cle.Loader(exe, auto_load_libs=False) + + def test_a_relocation_table_past_the_end_of_the_file_is_rejected(self): + # An otherwise well-formed 108-byte object whose .text points its relocation table at + # 0x4000000, so everything else the parser reads is in range. + exe = os.path.join(TEST_BASE, "tests", "x86", "coff_reloc_table_past_file.obj") + with self.assertRaisesRegex(cle.CLEInvalidBinaryError, "relocation table"): + cle.Loader(exe, auto_load_libs=False) + + def test_the_whole_object_still_loads(self): + # The bounds above are on what the header declares, so an object that declares only what + # it holds is unaffected. + exe = os.path.join(TEST_BASE, "tests", "x86", "fauxware.obj") + ld = cle.Loader(exe, auto_load_libs=False) + assert len(ld.main_object.sections) == 29 + assert len(ld.main_object.relocs) == 225 + def test_long_section_names_come_from_the_string_table(self): exe = os.path.join(TEST_BASE, "tests", "x86", "coff_long_section_names.obj") ld = cle.Loader(exe, auto_load_libs=False) @@ -59,6 +88,25 @@ def test_dir32_relocation_wraps_at_the_field_width(self): field_addr = section_vaddr(ld.main_object, ".text") assert ld.memory.load(field_addr, 4) == struct.pack("