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

["Carlos O'Donell" <carlos at redhat dot com>]

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.

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.