Re: preventable SIGSEGV when bad AT_SYSINFO_EHDR

[Markus Wichmann <nullplan@gmx.net>]

On Tue, Sep 19, 2017 at 09:46:19AM -0700, John Reiser wrote:
> __dls3() and friends in musl/ldso/dynlink.c should check Elf headers more carefully.
> I saw a SIGSEGV in decode_dyn() because vdso_base = ElfXX_auxv[{AT_SYSINFO_EHDR}].a_ptr
> pointed to a region that was all zero, and thus vdso.dynv == 0.  The operating system
> kernel is the only one who can perform a fork() or clone(), but other software can
> perform execve().  In my case that other software had a bug.  However, the blame
> for the SIGSEGV rests on __dls3() because it did not validate input data.  [This is
> the stuff of exploits.]  Calling a_crash() is OK; but a preventable SIGSEGV must be
> avoided, both directly and because it indicates a lack of secure implementation.
> 

How esoteric.

As far as I know, the aux headers come from the kernel and are
implicitly trusted because of that. If you have some userspace program
trying to do execve() without a kernel call, then that program needs to
correctly implement the aux headers. Mistrusting aux headers is as
sensible as mistrusting the kernel. For example, we fetch our user
credentials out of the aux headers in __init_libc().

But if your program emulates execve(), then how does security come into
play here? Security is only important if security domains are switched,
which a userspace program can't do (discounting kernel bugs, of course).
And so you're left with a program that might be able to exploit musl
into running arbitrary code in its own security domain. Newsflash: You
can already run arbitrary code in your own security domain. It's called
program execution. And if a buggy program was running with elevated
privileges and somehow could be tricked into running its execve()
emulation with attacker controlled data, then the problem has already
happened a long time ago, and you're merely asking us to fix the
symptoms.

In short, the quickest fix is to not emulate execve() but call the
kernel instead, hoping it won't screw up the ABI (if it does, abandon
ship, since all hope is lost).

Another fix would be to just link your programs statically. That way,
all the __dls*() functions don't run at all.

> It is [mostly] reasonable that __dls3() should trust that a non-zero vdso_base points to
> a region that is readable, is as big and as aligned as an ElfXX_Ehdr, and is const
> (no other thread is writing it, neither is any other process via a shared memory mapping);
> but after that ldso should check.
> 

No, if the value is set then it is correct by definition! Just as much
as AT_UID or AT_SECURE.

> In particular, these should be checked:
>   0 == memcmp(ELFMAG, &.e_ident[EI_MAG0], SELFMAG)
>   .e_machine matches the executing ldso
>   .e_ident[{EI_CLASS, EI_DATA}] match the executing ldso
>   .e_phnum != 0
>   .e_phentsize >= sizeof(ElfXX_Phdr);  and larger *IS ALLOWED*: derived classes, etc.

How do classes come into play in a file format? To my knowledge,
program headers have an explicitly defined layout, and a mismatching
phentsize is indicative of the program header in the file not being what
you thought it was.

>   .e_phnum * .e_phentsize is not too large  [loops that increment a pointer by .e_phentsize]
>   .e_phoff >= sizeof(ElfXX_Ehdr);  overlap of Ehdr and Phdr is a logical error
>   (.e_phoff + .e_phnum * .e_phentsize) < .st_size;  no access beyond EOF
> 

And where does that EOF check come from? We only know the start of the
kernel vDSO, but not its length. We can't stat() it, as it doesn't
really exist, it is merely injected into every process.

Ciao,
Markus