[arpitjain099]

Closes #3170. Closes #3171.

Both reports are the same shape, so they are fixed together: pyelftools reads the dynamic tables lazily and trusts that a table tag arrives with the companion tags describing it. When that does not hold, the exception escapes capa's extractors and ends the run, and capa reports nothing at all for a file the rest of which is perfectly analyzable. For a tool aimed at hostile files that is the wrong trade, and this file already takes the other side of it a few lines down, where an invalid relocation table is caught and skipped with a comment saying the ELF is corrupt.

iter_symbols and get_relocation_tables now go through two small helpers that log and return nothing when the segment cannot be read.

StopIteration is in the caught set alongside ValueError, RuntimeError and ELFError, and that detail is easy to get wrong. pyelftools reads the companion tags with bare next() calls, so the raw exception is StopIteration; because the extractors are generators, PEP 479 rewrites it into RuntimeError only at the generator boundary, which is above the call site. Catching RuntimeError alone leaves the bug in place, which I found by trying exactly that first.

Verified against real files rather than reasoned about. Starting from 055da8e6ccfe5a9380231ea04b850e18.elf_ in capa-testfiles, I made the two malformed variants the issues describe, one with the DT_GNU_HASH target zero-filled and one with DT_RELAENT relabelled to DT_DEBUG, and ran the two extractors over all three:

base     extract_file_export_names -> 19    extract_file_import_names -> 5
gnuhash  extract_file_export_names -> ValueError: max() iterable argument is empty
relaent  extract_file_import_names -> RuntimeError: generator raised StopIteration

After the change the unmodified file is still 19 and 5, and the two malformed files return partial results instead of raising. The stripped-hash file keeps all 19 exports, since those come from the section table.

The committed test does not need a sample. It drives both extractors with a dynamic segment whose table accessors raise, once per exception type, and asserts nothing propagates; against the current code all four fail with the exception escaping. I kept it sample-free deliberately so this does not require new binaries in capa-testfiles, but I am happy to contribute the two crafted files there if you would rather have end-to-end coverage.

pytest tests/test_elffile_features.py is 4 passed, with the four pre-existing FileNotFound failures from the test-data submodule not being checked out here. Ruff reports the same 7 errors and the same 2 files needing formatting before and after this branch, so it looks like my ruff differs from the pinned one rather than anything this change introduced.

[williballenthin]

i don't think these tests are necessary since they rely so heavily on fake/mocked data

[arpitjain099]

Agreed, I will drop them, and sorry for the slow reply on this.

They do not test much: the fake segment never runs DynamicSegment.__init__, so it has no header and no stream, and no pyelftools code is exercised. All it really pins is the exception tuple, and it is the only mocked test in the suite. Reachability is better shown with files.

You have the samples from #3170 and #3171 and posted counts for them, and I built the same variants while working on this. If you want end to end coverage I can send those files to capa-testfiles and add a sample driven test; otherwise the elffile.py change and the changelog line stand on their own.

[arpitjain099]

Dropped. tests/test_elffile_features.py is now byte-identical to master, so the PR is the extractor change plus a CHANGELOG line.

Adding real coverage needs the malformed binaries in capa-testfiles, which I cannot push to. Happy to attach the ones from #3170 and #3171 if you want them in there.

[Soremojinsen]

Thanks for picking these up. I filed both issues, so I ran this branch against the samples I used there. The relocation-table half looks right to me. The hash-table half stops the crash but drops data that is still recoverable, which is the trade-off I tried to argue against in #3170.

Counts below are unique names from extract_file_export_names / extract_file_import_names, as exports/imports. hello is the three-line C program from #3170, built with gcc and then strip --remove-section=.gnu.hash.

sample	master	this PR	recover from DT_SYMTAB
hello	18/3	18/3	18/3
hello, no .gnu.hash	crash	0/0	0/3
hello, no .gnu.hash, no section headers	crash	0/0	0/3
hello, DT_GNU_HASH pointing past EOF	18/0	18/0	18/3
hello, DT_RELAENT relabelled as DT_DEBUG	18/crash	18/0	18/0

One smaller thing: struct.error (hash chain past EOF) and AssertionError (_get_stringtable) can also escape here and are not in the tuple.

[arpitjain099]

Thanks for running it against your own samples, that table is more useful than anything I could produce from outside.

struct.error and AssertionError are both in the tuple now, with a case each.

On the hash-table half I don't disagree: your DT_SYMTAB column is the better outcome. I'd rather not do it here, though, because walking DT_SYMTAB without a count means picking where the table ends, and both candidate rules are heuristics that deserve their own review. So: land this as the thing that stops the abort, do the recovery in #3170 where you were already arguing for it. If the maintainers want one PR, I'll fold it in and would want your read on the end-of-table rule.