corellium / usbfluxd

Redirects the standard usbmuxd socket to allow connections to local and remote usbmuxd instances so remote devices appear connected locally.
GNU General Public License v2.0
352 stars 48 forks source link

Fix gethostbyname(3) data race #7

Closed meme closed 3 years ago

meme commented 3 years ago

Under certain conditions, such as a network failure, socket_connect_timeout will race to call gethostbyname(3) and end up dereferencing NULL:

if ((hp = gethostbyname(addr)) == NULL) {
    usbfluxd_log(LL_ERROR, "%s: unknown host '%s'", __func__, addr);
    return -1;
}

// Passes, data is fine
if (!hp->h_addr) {
    usbfluxd_log(LL_ERROR, "%s: gethostbyname returned NULL address!", __func__);
    return -1;
}

// ...
// Another thread calls gethostbyname, hp is now pointing to another instance (data race)
// ...

// BOOM
saddr.sin_addr.s_addr = *(uint32_t *) hp->h_addr;

The man page makes this apparent:

   The  functions  gethostbyname()  and gethostbyaddr() may return pointers to static data, which may be overwritten by
   later calls.  Copying the struct hostent does not suffice, since it contains pointers; a deep copy is required.

ASan:

==28115==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x561c82fb46e6 bp 0x7f8520a20ca0 sp 0x7f8520a20ae0 T357)
==28115==The signal is caused by a READ memory access.
==28115==Hint: address points to the zero page.
    #0 0x561c82fb46e6 in socket_connect_timeout /usbfluxd/usbfluxd/socket.c:153
    #1 0x561c82fba741 in check_remote_func /usbfluxd/usbfluxd/usbmux_remote.c:949
    #2 0x7f85da806258 in start_thread (/usr/lib/libpthread.so.0+0x9258)
    #3 0x7f85da5c75e2 in __GI___clone (/usr/lib/libc.so.6+0xfe5e2)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /usbfluxd/usbfluxd/socket.c:153 in socket_connect_timeout
Thread T357 created by T0 here:
    #0 0x7f85da889fa7 in __interceptor_pthread_create /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cpp:216
    #1 0x561c82fbaa4b in usbmux_remote_get_fds /usbfluxd/usbfluxd/usbmux_remote.c:967
    #2 0x561c82fc0ef2 in main_loop /usbfluxd/usbfluxd/main.c:129
    #3 0x561c82fc2870 in main /usbfluxd/usbfluxd/main.c:477
    #4 0x7f85da4f0b24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
strazzere commented 3 years ago

LGTM, much appreciated.