Prevent a NULL deref in _dwarf_loclists_fill_in_lle_head

[apetenchea]

Hi, I would like to suggest a small patch in dwarf_loclists.c, preventing a NULL deref in _dwarf_loclists_fill_in_lle_head() in case there is no loclists context.

I hit a reproducible crash in libdwarf when calling dwarf_get_loclist_c() on a DWARF5 object whose .debug_loclists data is missing/empty/invalid (in our case, relocations failed, so the loclist offset was garbage).

The code path returns DW_DLV_NO_ENTRY but then dereferences a NULL array pointer, causing a segfault.

This keeps the function consistent with other libdwarf call patterns. For a missing or malformed section it should return DW_DLV_ERROR and prevent a segfault. This patch returns an an error and callers can continue.

What do you think?

[davea42]

If the example that fails is not too large and not-proprietary, could you provide it so I can
add to the regression tests?

Thanks for the report and the fix.

[apetenchea]

If the example that fails is not too large and not-proprietary, could you provide it so I can add to the regression tests?

Thanks for the report and the fix.

It is unfortunately large, because we're exposing the DWARF sections through a Dwarf_Obj_Access_Interface_a, so that would mean sharing the whole elf/macho loader. Before opening this PR, I have spent some time trying to come up with a straightforward example, but stopped once I reached 300 lines.

The type of samples which can case problems are relocateble elf object files (.o) produced by the compiler before linking. I checked and the dwarfdump tool doesn't crash, but rather bails out, as it cannot validate the header of the .debug_loclists section (it sees junk bytes due to unprocessed relocations).

As for the project I'm working on, most of it is proprietary (Hex Rays). However, I will make sure to raise any issues upstream, in case I stumble upon anything in the future.

Thanks for accepting this patch!

[davea42]

The type of samples which can case problems are relocateble elf object files (.o) produced by the compiler before linking. I checked and the dwarfdump tool doesn't crash, but rather bails out, as it cannot validate the header of the .debug_loclists section (it sees junk bytes due to unprocessed relocations).

Following up on this.

When dwarfdump 'bails out' what message does it give you? Can you post that entire error message ,
please? It seems puzzling to me since the object checking is done by libdwarf and dwarfdump
honors libdwarf results. Ah. Since you are doing your own object reading
using Dwarf_Obj_Access_Interface_a it is up to you to
do object error checking, and yours is inadequate in some way.
I am guessing, but it seems likely that is the problem given your remarks.
libdwarf tries very hard to detect all bogus objects, but whatever you are using does not
try hard enough??

Does the above make sense to you? Wouldn't you be better advised to use
libdwarf to read the object?

I would be pleased to read any comments you feel free to make.

David Anderson

[apetenchea]

The message I get from dwarfdump:

./dwarfdump ERROR: ERROR: Failure reading CU header or DIE, corrupt DWARF: DW_DLE_VERSION_STAMP_ERROR: The version should be 5 but we find 96 instead.. Attempting to continue.

I am attaching the sample ELF file here. I think I got this from one of the libdwarf tests, so probably you have it already. sample.zip

The error is correct, .debug_loclists has an invalid DWARF5 header. On our side, we had to validate the header before going into get_loclist_c().

What you're suggesting makes sense, most probably we're abusing libdwarf's API. But in our case, using libdwarf to read the object is less flexible, because we're covering dwarf parsing out of ELF, MACHO and PE files, and we have custom loaders for each of these. Nothing wrong with the library here, just our use-case is more complex.
It may be easy for someone to misuse get_loclist_c() or forget to check the headers. This just turns the crash into a clear error for malformed inputs.