GCC-ASan makes libxcrypt's internal testsuite crash

[zackw]

I'm one of the developers of libxcrypt, which is a drop-in replacement for Glibc's libcrypt offering modern password-hashing algorithms. It implements, among other things, the functions crypt and crypt_r. In CI, we run our own testsuite with ASan enabled to weed out bugs.

In sufficiently recent versions of the libsanitizer shipped with GCC (version 10.2 is new enough; I don't know when this was added), there are interceptors for crypt and crypt_r. When we compile our own testsuite with -fsanitize=address, these interpose on the calls from our test programs to our implementations within our libcrypt.{a,so}. That's abstractly the Right Thing, but it doesn't work: we reach, for instance, __interceptor_crypt_r with the function pointer __interception::real_crypt_r (_ZN14__interception12real_crypt_rE) not yet having been initialized, so the program tries to call address 0 and crashes. (Debugger transcript at end of message.)

Is there something we ought to be doing in our code to make __interception::real_crypt_r get initialized, or is this a plain sanitizer bug?

---

Debugger transcript. libxcrypt was configured with CC='gcc -fsanitize=undefined,address' --disable-shared, after which make check compiles everything fine and reports a whole bunch of failures due to jumps to address 0, one of them being the special-char-salt testcase.

$ gdb --args test/special-char-salt
GNU gdb (Debian 10.1-1.7) 10.1.90.20210103-git
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from test/special-char-salt...
(gdb) r
Starting program: /home/zack/projects/xcrypt/libxcrypt/_build/test/special-char-salt 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
0x0000000000000000 in ?? ()
(gdb) bt
#0  0x0000000000000000 in ?? ()
#1  0x00007ffff7633dcf in __interceptor_crypt_r (
    key=0x55555557c0e0 "foobarbaz", salt=0x55555557c2a0 "$1$", 
    data=0x7fffffff6410)
    at ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:9598
#2  0x000055555557b2ad in main () at ../test/special-char-salt.c:864
(gdb) frame 1
#1  0x00007ffff7633dcf in __interceptor_crypt_r (
    key=0x55555557c0e0 "foobarbaz", salt=0x55555557c2a0 "$1$", 
    data=0x7fffffff6410)
    at ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:9598
9598    ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc: No such file or directory.
(gdb) disas
...
   0x00007ffff7633db3 <+99>:    call   0x7ffff761bc60 <__asan::QuickCheckForUnpoisonedRegion(__sanitizer::uptr, __sanitizer::uptr)>
   0x00007ffff7633db8 <+104>:   test   %al,%al
   0x00007ffff7633dba <+106>:   je     0x7ffff7633ef0 <__interceptor_crypt_r(char*, char*, void*)+416>
   0x00007ffff7633dc0 <+112>:   mov    %r12,%rdi
   0x00007ffff7633dc3 <+115>:   mov    %r14,%rdx
   0x00007ffff7633dc6 <+118>:   mov    %r13,%rsi
   0x00007ffff7633dc9 <+121>:   call   *0xdf1d9(%rip)        # 0x7ffff7712fa8 <_ZN14__interception12real_crypt_rE>
=> 0x00007ffff7633dcf <+127>:   mov    %rax,%r12
   0x00007ffff7633dd2 <+130>:   test   %rax,%rax
   0x00007ffff7633dd5 <+133>:   je     0x7ffff7633dfb <__interceptor_crypt_r(char*, char*, void*)+171>
...
(gdb) p _ZN14__interception12real_crypt_rE
$1 = (crypt_r_type) 0x0

[cryptomilk]

I've also hit this issue as python might call it:

(gdb) bt                                                                                      
#0  0x0000000000000000 in  ()
#1  0x00007ffff764f90f in __interceptor_crypt_r(char*, char*, void*) (key=0x7ffff41606a0 "", salt=0x7fffdca47560 "$6$xw6c2VXM1jXmR3Ci", data=0x7fffffff2f48) at ../../../../libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:9598
#2  0x00007fffdcae51da in crypt_crypt_impl.constprop () at /usr/lib64/python3.9/lib-dynload/_crypt.cpython-39-x86_64-linux-gnu.so
#3  0x00007fffdcae5333 in crypt_crypt () at /usr/lib64/python3.9/lib-dynload/_crypt.cpython-39-x86_64-linux-gnu.so
#4  0x00007ffff738f85c in cfunction_vectorcall_FASTCALL () at /lib64/libpython3.9.so.1.0
#5  0x00007ffff7386bee in _PyEval_EvalFrameDefault () at /lib64/libpython3.9.so.1.0

[fweimer-rh]

I think the interceptors are not really valid because the functions are not defined by dependencies of libasan, so dlsym with RTLD_NEXT cannot reliably find them.

Many distributions have moved to libxcrypt by now, and therefore can get a properly asan-instrumented libcrypt. The interceptors should probably no longer be included on GNU/Linux (or even plain Linux).

[fweimer-rh]

I submitted a patch to LLVM: https://reviews.llvm.org/D144073

[zackw]

@fweimer-rh Note that the interceptors for crypt and crypt_r are pickier about invalid function call arguments: quoting https://github.com/besser82/libxcrypt/commit/3aa82ccd3a3fecea7e6a9d0f9c85c56e2e04bb78#diff-964ef584f48a9f4feeae999451131ce52adc204e4f36e48fc00fdcffc3e4f85fR212

Our crypt() functions return NULL / a failure token, with errno set to EINVAL, when either the setting or the phrase argument is NULL. ASan's interceptors for crypt() instead crash the program when either argument is NULL -- this is arguably a better choice, but for compatibility's sake we can't change what our functions do. There is no way to disable interception of specific functions as far as I can tell. Therefore, these tests are skipped when compiled with ASan.

If you have any ideas for how libxcrypt could be modified to allow applications to opt into the pickier behavior, perhaps by using one or more of -fsanitize=address, -fsanitize=undefined, -D_FORTIFY_SOURCE, please make suggestions (in the form of either a new issue or a PR) over at https://github.com/besser82/libxcrypt .