Re: [PATCH] Avoid mapping past end of shared object (BZ #18685)

[pinskia at gmail dot com]

> On Jul 17, 2015, at 12:02 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> 
>> On 07/16/2015 11:28 PM, Siddhesh Poyarekar wrote:
>>> On Thu, Jul 16, 2015 at 11:59:18AM -0400, Carlos O'Donell wrote:
>>> This is not the right fix for this problem. The right fix has not
>>> been attempted because it involves someone doing some real leg work
>>> to gather consensus. This fix adds complex checking in ld.so for
>>> minimal gain, and eventually you'll get a debuginfo file that is
>>> different again in some odd way.
>> 
>> This is not specifically about being able to read debug files, nor is
>> it about ldd.  It just happens to be ldd (ld.so [--verify|--list])
>> that crashed, but the offending code is bang in the middle of generic
>> ld.so code that can potentially be exploited when running arbitrary
>> binaries.  While it is true that one should not run arbitrary code
>> anyway, it shouldn't be an excuse for not fixing bugs.  I don't see
>> the point of not adding such checks as they come up; performance is an
>> excuse made quite regularly, but what is the actual cost of such
>> checks?
> 
> Is this really a bug?
> 
> The cost of the checks is more than just performance, but also maintenance
> and support of that code into the future. It is about testing that code,
> making sure we exercise those weird conditional failure paths which we
> thought would be a good thing to add. Each of these checks is an additional
> complication, added branch, and conditional in already complicated code.
> I would like to see the core loader as simple as possible and to expect
> as correct as possible input.

Yes but the cost is cheaper than someone accidentally doing a dlopen of a file and causing a crash rather than retuning malformed elf file. I suspect that is a way how to reproduce this failure. 

Thanks,
Andrew

> 
> If I ask you for a test case for bug 18685, do we have one we can checkin
> to test this code? That goes a long way to helping maintain this code.
> 
> If ldd should not be run on untrusted input... then why are we looking
> at this patch? I think the fact on the ground is that ldd *is* run on
> untrusted input and eu-ldd isn't ready, and we face the grim specter of
> security issues. However, if we're talking security issues, there are
> a million ways I could craft relocs to overwrite and assemble ROP
> widgets to execute a custom program that would run crafty exploits.
> Running ldd is equivalent to ./app.
> 
> See: "DEFCON 20: Programming Weird Machines with ELF Metadata"
> https://www.youtube.com/watch?v=YgtxxLCVD-o
> 
> So again, we can never support real ld.so running or processing untrusted
> input? The real fix becomes: Don't use ld.so, don't process relocs,
> don't run anything, just analyze.
> 
>> To be clear, I am not against having an eu-ldd, but that shouldn't be
>> an excuse for not patching ld.so.  Things that don't crash on eu-ldd,
>> should not crash on ld.so.
> 
> To fix or not to fix ld.so should be done on the merit of the individual
> change.
> 
> I disagree that eu-ldd and ld.so should both be equally robust against
> bad input. Just consider the myriad of problems the binary could have,
> the fuzzing we put libelf through, all of that is costly verification
> that ld.so should not do.
> 
>> Oh, and did I mention that eu-ldd (and most of elfutils) should
>> ideally be written in an interpreted language (cough*python*cough) so
>> that we reduce the attack surface on them?
> 
> How does python reduce the attack surface? I've already started eu-ldd
> in C because it needs to know quite a bit about the low-level pieces
> and it was easiest for me to do this in C (headers, constants etc).
> However, from a "independent implementation" perspective, writing it
> in python would make it truly orthogonal from the C version.
> 
> Any other benefits to python?
> 
> Cheers,
> Carlos.