[Bug target/78631] New: [Intel MPX] libmpxwrappers shared library leads to a non-bounds-preserving memcpy()

["dmitrii dot kuvaiskii at tu-dresden dot de" <gcc-bugzilla at gcc dot gnu dot org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78631

            Bug ID: 78631
           Summary: [Intel MPX] libmpxwrappers shared library leads to a
                    non-bounds-preserving memcpy()
           Product: gcc
           Version: 6.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dmitrii.kuvaiskii@tu-dresden.de
  Target Milestone: ---

The libc-wrappers library for Intel MPX, libmpxwrappers, has incorrect
implementations of memcpy and mempcpy functions.

When any program calls memcpy(), the compiler pass substitutes it to
__mpx_wrapper_memcpy(), so the call is redirected into libmpxwrappers.
Unfortunately, __mpx_wrapper_memcpy() has a buggy implementation which leads to
the loss of bounds information and can miss real out-of-bounds violations.

(The following was tested on gcc 6.1.0, but the code for 6.2.0 and in the
master branch seems to be the same, thus I believe this bug persists in newer
versions as well. The affected code is in libmpx/mpxwrap/mpx_wrappers.c)

So, this is the implementation of __mpx_wrapper_memcpy():

void * __mpx_wrapper_memcpy (void *dst, const void *src, size_t n) {
  return __mpx_wrapper_memmove (dst, src, n);
}

__mpx_wrapper_memmove() is:

void *__mpx_wrapper_memmove (void *dst, const void *src, size_t n) {
  ...
  __bnd_chk_ptr_bounds (dst, n);
  __bnd_chk_ptr_bounds (src, n);
  ... call real memmove() ...
}

Now, when it is compiled into a shared libmpxwrappers.so library, the following
asm results:

<__mpx_wrapper_memcpy>:
 1f50:  sub    $0x8,%rsp
 1f54:  bnd callq ad0 <__mpx_wrapper_memmove@plt>
 1f5a:  add    $0x8,%rsp
 1f5e:  bnd retq

<__mpx_wrapper_memmove@plt>:
 ad0:   jmpq   *0x201c5a(%rip) # 202730 <_GLOBAL_OFFSET_TABLE_+0x18>
 ad6:   pushq  $0x0
 adb:   jmpq   ac0 <_init+0x20>

<__mpx_wrapper_memmove>:
 ... actually performs bounds checking ...

Pay attention to the jmpq instruction (at line ad0) which indirectly calls
__mpx_wrapper_memmove() through PLT/GOT. Since this instruction has no BND
prefix, all BND0-3 registers will be INITed. (This of course depends on the
BNDPRESERVE bit; since BNDPRESERVE is 0 by default this is the behavior we
observed.)

In a nutshell: the PLT/GOT resolving mechanism puts a plain jmp instruction
instead of a "bnd jmp" version --> this INITs registers BND0-3 in
__mpx_wrapper_memmove() which actually performs bounds checking --> a real
out-of-bounds bug in the program is missed.

Simple workaround would be to copy-paste bounds-checking code
("__bnd_chk_ptr_bounds ...") inside __mpx_wrapper_memcpy(). Then the checks
will definitely happen before any registers are nullified.

A workaround that I used was to set BNDPRESERVE=1 (then my program bug was
correctly detected). But this worked only for my tiny program which had no
other legacy libraries.

A real solution would require re-building libmpxwrappers with a correct PLT/GOT
resolution which uses BND-prefixed jumps.