[Toybox] readelf questions.

[Rob Landley]

On 2/3/20 10:15 PM, enh wrote:
> On Tue, Jan 28, 2020 at 7:11 PM Rob Landley <rob at landley.net> wrote:
>>
>> Is reading 64 bit binaries from 32 bit systems interesting? Because struct sh
>> and struct ph have "int" and "long" values, which are the same on 32 bit.
>> (long long is the one that's always 64 bit.)
> 
> i certainly intended to support that combination. i've actually since
> added CTS tests that use `readelf -S` output, but obviously not on
> files with values big enough to break this.

$ git diff toys/*/readelf.c | diffstat
 readelf.c |  330 +++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 166 insertions(+), 164 deletions(-)

But I got distracted by $DAYJOB before finishing.

> i've sent the obvious patch.
> 
>> Last elf plumbing I did (file?) I carefully made sure every access wouldn't go
>> off the end of the mmap. This doesn't seem to ever check that? (ph.filesz could
>> be zero then you're feeding -1 to printf, etc...)
> 
> like i said, i haven't fuzzed this at all, so there's doubtless _some_
> checks missing. and, yes, that seems to be one. (and a sign [no pun
> intended] that all this should be switched to unsigned.) i'll send a
> patch for that tomorrow night if you haven't already done it by then.

I've learned my lesson not to try cleaning up commands until the submitter is
done with them. My bad for starting early this time...

I deleted my changes so I could apply yours. (Attached in case you're curious,
but otherwise I'm sticking to shell stuff this release.)

> anywhere there's a dereference should be protected: search for
> references to TT.size. there aren't as many as you might expect to see
> because get_sh() and get_ph() won't return a bad section or program
> header.

I was mostly converting it to auto-advancing elf_get(char **pos, int len) with
elf_int() and elf_long() wrappers on top of that to handle the 4 and 8 byte
common cases. (There were a few 2 bytes, and the rest were *ll++.)

That way it didn't have to do the traversal math multiple times in parallel by
hand. With that and a str_eat() function that was like strstart requiring a full
match and rounding up to 4 I think I had most of it converted over. (Which
avoided a function pointer in the globals...)

>> Should work on non-pathological ELF files, of course... :)
> 
> sadly i didn't test with chrome's .so files or i'd have caught this :-)

*shrug* I hadn't gotten to a point where it compiled again yet, I was just
reading the code...

Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: elf.patch
Type: text/x-patch
Size: 19542 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20200204/5b31e4ea/attachment-0003.bin>