diff --git a/cle/backends/elf/symbol.py b/cle/backends/elf/symbol.py index bd0788d24..4976518ac 100644 --- a/cle/backends/elf/symbol.py +++ b/cle/backends/elf/symbol.py @@ -1,5 +1,6 @@ from __future__ import annotations +from elftools.elf.constants import SHN_INDICES from elftools.elf.enums import ENUM_ST_INFO_TYPE from cle.address_translator import AT @@ -31,9 +32,15 @@ def __init__(self, owner, symb): sec_ndx, value = symb.entry.st_shndx, symb.entry.st_value + # pyelftools decodes SHN_UNDEF, SHN_ABS and SHN_COMMON to strings and passes every other st_shndx + # through as an int. Values from SHN_LORESERVE up are reserved tags, such as the processor-specific + # SHN_X86_64_LCOMMON, and do not index the section header table. + reserved_ndx = isinstance(sec_ndx, int) and sec_ndx >= SHN_INDICES.SHN_LORESERVE + section = None if reserved_ndx or isinstance(sec_ndx, str) else sec_ndx + # A relocatable object's symbol's value is relative to its section's addr. - if owner.is_relocatable and isinstance(sec_ndx, int): - value += owner.sections[sec_ndx].remap_offset + if owner.is_relocatable and section is not None: + value += owner.sections[section].remap_offset super().__init__( owner, maybedecode(symb.name), AT.from_lva(value, owner).to_rva(), symb.entry.st_size, self.type @@ -42,14 +49,17 @@ def __init__(self, owner, symb): self.version = None self.binding = symb.entry.st_info.bind self.is_hidden = symb.entry["st_other"]["visibility"] == "STV_HIDDEN" - self.section = sec_ndx if not isinstance(sec_ndx, str) else None + self.section = section self.is_static = self._type == SymbolType.TYPE_SECTION or sec_ndx == "SHN_ABS" self.is_common = sec_ndx == "SHN_COMMON" self.is_weak = self.binding == "STB_WEAK" self.is_local = self.binding == "STB_LOCAL" self.is_import = sec_ndx == "SHN_UNDEF" and self.binding in ("STB_GLOBAL", "STB_WEAK") - self.is_export = (self.section is not None or self.is_common) and self.binding in ("STB_GLOBAL", "STB_WEAK") + # A reserved st_shndx names no section, but the symbol is still defined here, the way a SHN_COMMON + # one is: SHN_X86_64_LCOMMON is what a large common symbol gets instead of SHN_COMMON. + defined_here = self.section is not None or self.is_common or reserved_ndx + self.is_export = defined_here and self.binding in ("STB_GLOBAL", "STB_WEAK") @property def subtype(self) -> ELFSymbolType: diff --git a/cle/backends/pe/pe.py b/cle/backends/pe/pe.py index c6fab9f5d..95fe43a81 100644 --- a/cle/backends/pe/pe.py +++ b/cle/backends/pe/pe.py @@ -195,12 +195,7 @@ def __init__( # Go binaries keep a full function table even when stripped self.gopclntab = register_gopclntab_symbols(self) - self.is_dotnet = ( - self._pe.OPTIONAL_HEADER.DATA_DIRECTORY[ - pefile.DIRECTORY_ENTRY["IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR"] - ].VirtualAddress - != 0 - ) + self.is_dotnet = self._meta_dd("IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR") is not None _pefile_cache = {} @@ -511,10 +506,22 @@ def _meta_pe_context(self) -> tuple[pefile.PE, int, bool, int]: ptr_size = 8 if is_64 else 4 return pe, base, is_64, ptr_size - def _meta_dd(self, name: str) -> pefile.Structure | None: - """Return a data directory entry if it has a nonzero VirtualAddress and Size, else None.""" + def _meta_dd(self, name: str): + """ + Return the named data directory entry if the image has one with a nonzero VirtualAddress and Size. + + The return type is inferred rather than declared: pefile fills a data directory entry in from the format + string it parsed, so the pefile.Structure this used to promise declares neither VirtualAddress nor Size and + hides both from every caller. + """ idx = pefile.DIRECTORY_ENTRY[name] - dd = self._pe.OPTIONAL_HEADER.DATA_DIRECTORY[idx] + data_directory = self._pe.OPTIONAL_HEADER.DATA_DIRECTORY + # NumberOfRvaAndSizes may be smaller than the 16 directories the format defines - EFI stub images + # commonly declare 6 - so pefile parses a short DATA_DIRECTORY. A directory past the end is absent, + # which means the same thing a zero VirtualAddress means. + if idx >= len(data_directory): + return None + dd = data_directory[idx] if dd.VirtualAddress and dd.Size: return dd return None diff --git a/tests/test_elf_symbols.py b/tests/test_elf_symbols.py new file mode 100644 index 000000000..0ec30703a --- /dev/null +++ b/tests/test_elf_symbols.py @@ -0,0 +1,45 @@ +#!/usr/bin/env python +from __future__ import annotations + +import os + +import cle +from cle.backends.elf.symbol import ELFSymbol + +TESTS_BASE = os.path.join(os.path.dirname(os.path.realpath(__file__)), "..", "..", "binaries", "tests") + + +def test_large_common_symbol(): + """ + An st_shndx from SHN_LORESERVE up is a tag, not a section header table index. + + large_common.o is built with -mcmodel=medium, which puts a common symbol too large for the small + code model at SHN_X86_64_LCOMMON (0xFF02) instead of SHN_COMMON. Subscripting the section list + with that used to raise IndexError before the object finished loading. + """ + ld = cle.Loader(os.path.join(TESTS_BASE, "x86_64", "large_common.o"), auto_load_libs=False) + assert isinstance(ld.main_object, cle.ELF) + assert ld.main_object.is_relocatable + + symbol = ld.main_object.get_symbol("big_buffer") + assert isinstance(symbol, ELFSymbol) + # The tag names no section, so the symbol gets none and no section remap offset either. + assert symbol.section is None + assert not symbol.is_common + # It still defines big_buffer for anything linking against this object. + assert symbol.is_export + + +def test_common_symbol(): + """An ordinary SHN_COMMON symbol keeps naming no section, and stays an export.""" + ld = cle.Loader(os.path.join(TESTS_BASE, "x86_64", "decompiler", "gzip.o"), auto_load_libs=False) + symbol = ld.main_object.get_symbol("ofname") + assert isinstance(symbol, ELFSymbol) + assert symbol.is_common + assert symbol.section is None + assert symbol.is_export + + +if __name__ == "__main__": + test_large_common_symbol() + test_common_symbol() diff --git a/tests/test_pe.py b/tests/test_pe.py index 7b32cabb9..d228d33cd 100644 --- a/tests/test_pe.py +++ b/tests/test_pe.py @@ -300,6 +300,20 @@ def test_load_binary_larger_than_highest_address(self): assert ld.main_object.max_addr == 0x44F02D assert ld.all_objects[1].min_addr == 0x500000 + def test_short_data_directory(self): + # NumberOfRvaAndSizes may be smaller than the 16 directories the format defines, and the + # trailing directories are then simply absent. EFI stub images commonly declare 6, which is + # what this one does; indexing the IAT (12) or the .NET descriptor (14) has to cope. + efi = os.path.join(TEST_BASE, "tests", "x86_64", "efi_short_data_directory.efi") + ld = cle.Loader(efi, auto_load_libs=False) + + assert isinstance(ld.main_object, cle.PE) + assert [sec.name for sec in ld.main_object.sections] == [".text", ".data"] + assert ld.main_object.entry == 0x1000 + assert not ld.main_object.deps + # The .NET descriptor is directory 14, past the end of this image's directories. + assert not ld.main_object.is_dotnet + def test_loading_incomplete_pe_file(self): exe = os.path.join( TEST_BASE, "tests", "i386", "windows", "a94bbeed0ef51db3d3964bb0cc2cbed0adab0e47997d88f34daa92faa1a91e8a"