From ecadc830656ea1019d303221b8ca98db3a6ca050 Mon Sep 17 00:00:00 2001 From: Yan Date: Wed, 2 Sep 2026 04:21:24 +0000 Subject: [PATCH 1/2] COFF: skip a relocation whose field lies outside its own section Coff._add_relocs took the patch address as section.PointerToRawData plus reloc.VirtualAddress and registered a relocation there without checking it. Neither bound was tested, and the two fail differently. A field past the end of the file crashes. The backend maps the object as one backer covering the file, so CoffRelocationDIR32.value asks Clemory for four bytes at an address nothing maps, and cle.Loader(..., perform_relocations=True) raises KeyError out of Clemory.load. A field merely past the end of its own section does not crash, and that is the worse half. Every offset in the file is mapped, so the store lands wherever the arithmetic points -- another section's raw data, the relocation table, the symbol table -- and the load returns normally with those bytes rewritten. Check both bounds where the relocation is registered rather than in relocate(). A relocation that cannot be applied should not reach self.relocs at all: it is handed to the symbol resolver, it can produce an extern symbol for a field that will never be written, and it is visible to every consumer that iterates an object's relocations. It is also where the PE backend drops a section whose raw data the file does not hold. The field's width comes from struct.calcsize on the relocation class's PACK_FORMAT -- four bytes normally, eight for ADDR64, two for SECTION -- so a four-byte field starting on the last byte of a section is out of bounds, which a bound on the start offset alone would miss. PACK_FORMAT is declared on CoffRelocation rather than on Relocation, so RELOC_CLASSES is annotated with the class it actually holds. This leaves the section mapping loop alone. Bounding a section's raw data by the size of the file is #806; the two compose, because _add_relocs walks self._coff.sections itself and would still register the relocations of a section that loop has skipped. Two details keep this bound correct against the other open COFF branches, and change nothing on this one. The section comes out of self._coff.sections by index rather than off the loop variable. Both name the same object here, by the definition of enumerate. #764 rewrites this loop to walk indices and drops the variable, and the two branches merge with no textual conflict, so with both applied and the loop variable read _add_relocs raises NameError on the first relocation of a supported type. Of the five COFF objects angr/binaries tracks that this backend loads, four carry such a relocation and stop loading; the fifth has none. #804 is stacked on #764 and carries the same rewrite. The file-size half of the bound is taken against self._image_vmem, the bytes the backend maps, rather than against self._data. Here the two are the same object: _image_vmem is assigned from _data in __init__, never rebound, and cle defines no subclass of Coff. #804 places a section whose file offset does not satisfy its alignment past the end of the file and extends the image to cover it, so a relocation into a moved section is past len(self._data) and inside the image, and bounding on the file would skip it. With both applied and the file used, x86/fauxware.obj keeps 177 of its 225 relocations and x86_64/fauxware.obj 66 of 126, and the test below asserting 225 fails. --- cle/backends/coff.py | 17 ++++++++++++++++- tests/test_coff.py | 19 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/cle/backends/coff.py b/cle/backends/coff.py index 556ca93b..815d82dd 100644 --- a/cle/backends/coff.py +++ b/cle/backends/coff.py @@ -413,7 +413,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 +514,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..b534986e 100644 --- a/tests/test_coff.py +++ b/tests/test_coff.py @@ -59,6 +59,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(" Date: Wed, 2 Sep 2026 05:01:32 +0000 Subject: [PATCH 2/2] COFF: reject a header table the file does not hold CoffParser._parse reads three tables at offsets and counts that come out of the file, and bounds none of them. A truncated or malformed object therefore leaves cle.Loader by an exception a caller cannot name: struct.error from the string table size read, and ValueError from ctypes for the section and relocation tables. Neither is a CLEError, so nothing catching CLEError catches them. Bound all three against the size of the file and raise CLEInvalidBinaryError naming the field, the bytes it wanted and the size of the file. The message follows the register of the PE backend's out-of-bounds section warning. A table with no entries is exempt, because nothing dereferences its pointer: a section declaring zero relocations and a PointerToRelocations past the end of the file loads on master and has to keep loading. The bounds change no acceptance decision. What changes is the exception: a rejection that was struct.error or ValueError is now CLEInvalidBinaryError. One bound covers two of the reads. The string table begins on the byte after the last symbol, so a file that holds the four-byte string table size also holds every symbol. Loading all 16677 truncations of x86/fauxware.obj, from zero bytes to the whole file: on master 15457 of them -- every length from 20 to 15476 -- fail at the string table size read and not one reaches the symbol table read. 15477 is the first length that holds that size field, and the 1200 lengths from there up load. A separate bound on the symbol table would be unreachable. Those same 1200 lengths still load with these bounds applied. The 15457 that failed now fail as CLEInvalidBinaryError, 1160 of them on the section table and 14297 on the symbol and string table. Malformed here is fatal rather than skippable, because the parser has no partial product to hand back. _add_relocs indexes self._coff.symbols by an index taken from the file, so a short symbol list turns a truncated object into an IndexError somewhere with no information about why. That index is itself unbounded on both revisions and stays that way here; bounding it is a separate change. The constructor already treats an unusable COFF as fatal for an unsupported machine type and for a /GL object, so this replaces an accidental exception with a named one rather than adding a new failure. Not bounded: CoffFileHeader.from_buffer_copy on the first line of _parse, which still raises ValueError for 18 of the 20 truncations shorter than the twenty-byte header. The other two never reach the COFF backend at all. That read takes no field from the file. Whether a two-byte file should reach the parser is Coff.is_compatible's question, since it claims a file on its first two bytes, and it is a separate change. --- cle/backends/coff.py | 30 ++++++++++++++++++++++++++++-- tests/test_coff.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/cle/backends/coff.py b/cle/backends/coff.py index 815d82dd..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) diff --git a/tests/test_coff.py b/tests/test_coff.py index b534986e..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)