Re: Alternative libio vtable hardening approach

[Kees Cook <keescook at chromium dot org>]

On Tue, May 31, 2016 at 6:07 AM, Florian Weimer <fweimer@redhat.com> wrote:
> I have implemented a completely different approach to vtable hardening.
>
> The basic idea is to put all libc vtables into a single array, and then
> check whether the vtable pointer is within that array.
>
> There is a new flag, IO_accept_foreign_vtables, which is initially false and
> set to true to indicate that vtables may have been defined outside of libc.
> This allows us to preserve full backwards compatibility.
>
> The attached patch is not completely finished.  We need to disable vtable
> validation on both sides of a static dlopen boundary because the vtables are
> not shared across the boundary.  This is what causes dlfcn/tststatic2 to
> fail.  We also may have to set the IO_accept_foreign_vtables flag for
> additional constructor functions (although _IO_file_init may be sufficient,
> based on what I have seen in GCC).
>
> A future optimization would change the virtual function calls to compare the
> function pointer in the vtable against an expected function pointer.  Only
> if that comparison fails, the vtable pointer is validated.  This will allow
> us to inline parts of xsputn into vfprintf, especially if we change the
> various implementations to share more code.
>
> At this point, I'm mainly interested in comments whether the use of the flag
> is acceptable from a security perspective.  I expect that if you can set the
> flag and overwrite vtable pointers, you already have substantial control
> over what the process does.  It is also likely that you would able to reset
> the pointer guard variable, disabling the hardening in Kees' patch.

Well, this is certainly better than not having it, and the on/off
switch isn't in the FILE structure, so I would think this should be of
a similar protection level. Though, it'd be nice if a process could
opt-out for its entire lifetime. Right now, any call to _IO_file_init
disables the protection.

(Also, is the call to _IO_new_file_init in _IO_new_popen wrong? Should
that be the internal one instead?)

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security