Stop indexing PE and ELF header tables past their declared length - #732
Stop indexing PE and ELF header tables past their declared length#732zardus wants to merge 2 commits into
Conversation
|
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS Validation record for head Reproducers, both through
Behavior deltas beyond the crash, both checked:
Caveats: two jobs needed a rerun for reasons outside this change, and both pass on the rerun. The images that first showed this came from a corpus sweep and are not published here. The PE side is reproducible on public material — OpenWrt 24.10.2 EFI-stub release kernels for x86-64, aarch64 and loongarch64 all declare 6 data directories; the x86-64 one is sha256 Re-measured at
Not re-run at this head: the full suite, the lint and type comparison, the pre-commit hooks and the fixture surveys above. Those figures are the ones measured at Re-keyed 2026-08-28. The opening line named Re-keyed 2026-08-29, second time. The opening line named CI disposition. The previous head The two
The old merge base fails identically to the old head, which is what rules the diff out. The third row of each The third failure was the pinned fixture branch. Typecheck. Platform legs. On earlier heads Hosted checks at this head are reported when the run reaches a terminal state. Neighbouring pull requests. Two open cle pull requests touch these lines. cle#756 carries a commit with the same What did not run. The workspace-wide gate needs a Re-keyed 2026-09-04. Head is now The Hosted CI at this head, read live 2026-09-04: 20 terminal checks, every one |
6cfc4d2 to
27a2c9d
Compare
|
Corpus decompilation diffs can be found at angr/dec-snapshots@master...angr/cle_732 |
c99ff82 to
c857b4e
Compare
|
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS Full load report for Before — both objects abort mid-load with cle at the merge base, eac0e55After — both load, with this change, cf834d8 |
b9bc8de to
cf834d8
Compare
Loading aborted with IndexError on two header-controlled numbers that were used as list indices without being checked against the list. PE: NumberOfRvaAndSizes states how many data directories an image has, and fewer than 16 is legal - EFI stub images commonly declare 6 - so pefile parses a short DATA_DIRECTORY. The IAT (12) and .NET descriptor (14) lookups indexed it with fixed constants. An index past the end means the image does not have that directory, which is what a zero VirtualAddress already means, so treat it the same way. Both lookups now go through _meta_dd, the one place that decides whether a directory is present, so is_dotnet no longer keeps its own copy of that decision; it now also wants a nonzero Size, the way every other directory does. ELF: st_shndx values from SHN_LORESERVE up are reserved tags, not section header table indices. pyelftools decodes only SHN_UNDEF, SHN_ABS and SHN_COMMON into strings and passes the rest through as ints, so a symbol in the processor-specific SHN_X86_64_LCOMMON indexed the section list. Such a symbol names no section, so it gets neither a section nor a remap offset. It stays an export: SHN_X86_64_LCOMMON is what a large common symbol gets instead of SHN_COMMON, and cle already exports those. Both cases need an input no binary in angr/binaries had, so both fixtures are new there: efi_short_data_directory.efi, a UEFI application whose optional header declares six directories, and large_common.o, built by gcc -mcmodel=medium. Each reproduces its IndexError on the unfixed loader. Requires the angr/binaries branch feature/short-data-directory-and-lcommon. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Typecheck job scores each changed file against master's copy of it: badness is (10*errors + warnings)/lines and must not increase. Its environment installs types-pefile, whose stub types OPTIONAL_HEADER.DATA_DIRECTORY as a list of entries carrying VirtualAddress and Size. _meta_dd declared pefile.Structure, the base class that has neither, which threw that away for every caller: 39 of pe.py's 52 errors are a caller reading VirtualAddress or Size off the result. That is what makes this branch red. It adds no diagnostic of its own - it scores the same 52 errors as master - but its pe.py is 26 lines shorter than master's, and the same error count over a smaller line count is a regression by that measure. Leaving the return type to inference gives the entry type where the stub is installed and an unknown one where it is not, so the callers type-check and nothing about them changes at run time. pe.py goes from 52 errors to 13. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
cf834d8 to
43c897a
Compare
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS
Problem
cle.Loaderaborts withIndexErroron two numbers a container's own header controls, and in both cases the whole object is lost over one field that could have been ignored.tests/x86_64/large_common.o, built-mcmodel=medium, placesbig_bufferatSHN_X86_64_LCOMMON(0xFF02):tests/x86_64/efi_short_data_directory.efideclares 6 data directories rather than 16:Root cause
ELFSymbol.__init__treated any integerst_shndxas a section header table index:An
st_shndxfromSHN_LORESERVEup is a reserved tag, not an index, but pyelftools decodes onlySHN_UNDEF,SHN_ABSandSHN_COMMONto strings and hands the rest back as plain ints, so0xFF02subscripts a list of a dozen sections.PE._meta_ddhas the same "the index is always valid" assumption inself._pe.OPTIONAL_HEADER.DATA_DIRECTORY[idx], over a list pefile sizes fromNumberOfRvaAndSizes. The optional header may declare fewer than the 16 the format defines, and EFI-stub images commonly declare 6, so the IAT and .NET descriptor lookups index past the end.Fix
Each backend checks its table before indexing it. A reserved
st_shndxnames no section, so its symbol gets none and no remap offset while staying an export, exactly as aSHN_COMMONsymbol does; the reserved tags are recognised as "not a section index" and not decoded into individual meanings. A directory past the end of a shortDATA_DIRECTORYis absent, the way a zeroVirtualAddressalready is, andis_dotnetgoes through_meta_ddinstead of indexing directly. That also makesis_dotnetrequire the nonzeroSizethe helper has always required, which is how_meta_com_descriptoralready reads directory 14, so cle stops answering "this image is .NET" and "this image has no CLR header region" about the same file.CFGFastis the only consumer; it reads the flag to choose between the .NET and the native scanning defaults.Testing
tests/test_elf_symbols.py::test_large_common_symbolassertssymbol.section is Nonewithsymbol.is_exportguarding the semantics,::test_common_symbolpins theSHN_COMMONcontrol ontests/x86_64/decompiler/gzip.o, andtests/test_pe.py::TestPEBackend::test_short_data_directoryloads the EFI image. Both regressions load a fixture with the header shape they exercise rather than rewriting a field in a copy of one that has a different shape; both fixtures are real toolchain output.sync: angr/binaries#205
Validation: #732 (comment)
session: sharpen