Make DynELF more robust to different base addresses

[gsx]

The semantics of some fields in ELF headers (notable examples are the entry point, p_vaddr for PHDRs, and d_ptr in Elf{32,64}_Dyn, but there are probably more) depend on the ELF Type. 'EXEC' type ELFs (non-PIE executables) will use actual addresses, while 'DYN' type ELFs (shared libraries or PIE executables) will use offsets from the ELF base address. However, the run-time linker may then fix up these offsets to be real addresses. Currently DynELF checks for this by decreeing that values smaller than 0x400000 are offsets, while above that the value is likely to be an actual virtual address.

This works for executables produced with gcc, as the default linker script that comes with the GNU linker, ld, sets the image base address to 0x400000 (or 0x8048000 for 32 bit). However, other linkers might choose different values. For example when linking with the LLVM linker, lld, and using its default linker script the base address is 0x10000.

The result in the lld example is that DynELF adds the base address to pointer values again, ending up with addresses in the 0x20000-0x30000 range, and so my exploit goes into some sort of quasi-infinite loop trying to leak from addresses which are not mapped.

The proposed change instead determines when to add the base address to the value read from these fields by looking at the Type field in the ELF header, and if that indicates that the value may be an offset, making a similar guess as before, but instead of hard-coding the value 0x400000, it uses the library base address.

My current understanding is that the fields mentioned above may contain offsets if and only if the ELF Type is 'DYN'. I am basing this on some superficial reading and a few experiments, so I may easily be wrong.

Testing this change does not seem straightforward to me. I have cooked up a very simple example, for which the new version seems to work with all combinations of PIE, RELRO, and linker. My simple test case can be seen here.

To me this feels like a bugfix and so the pull request is against the stable branch, but I would understand if someone was to argue that the nature of this fix is such that it should be considered an enhancement instead.

[zachriggle]

Thanks for making this pull request!

You are correct that this should be made against the dev branch. You should only need to change the target. Changes should only be merged to stable when they are bugfixes -- I'd view this more as an enhancement.

Separately, I'd much prefer to just leak the header field rather than trying to the DYN vs EXEC state down. We're already leaking the header generally (since we scan-down for \x7fELF), so performing a single additional leak to get the Ehdr.e_type would be my preferred mechanism for this.

This would also simplify the current logic of _make_absolute_ptr since we can just refer to the value that we leaked (which won't be re-leaked, but is cached via the MemLeak object). This would also remove the need to "guess" at all.

[gsx]

Entirely fair points, the both of them. This was originally meant to be a one line monkey patch, and I seem to have lost the big picture for a while. Anyhow, now it should be against the right branch and it works by leaking the ELF header instead of awkwardly passing around boolean flags. I have once again tested using the (updated) script above. It seems to work on all combinations of 32/64 bits, RELRO, PIE, and linker. (32-bit PIE binaries linked with LLD could not be tested because of an outstanding bug in LLD.)

[TethysSvensson]

@zachriggle: Could you put a label on this? I am guessing the enhancement label?