Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MemorySanitizer misses uninitialized argument of memcpy() #1296

Open
mpg opened this issue Aug 18, 2020 · 1 comment
Open

MemorySanitizer misses uninitialized argument of memcpy() #1296

mpg opened this issue Aug 18, 2020 · 1 comment

Comments

@mpg
Copy link

mpg commented Aug 18, 2020

MemorySanitizer fails to catch when the src argument of memcpy() depends on an uninitialized value. (This may also be the case with other arguments of memcpy() and other library functions, I didn't test.)

Steps to reproduce:

cat <<EOF > main.c
#include <stdlib.h>
#include <string.h>

static char a[16];
static const char b[16];

int main(void)
{
    size_t *offset = malloc(sizeof(size_t));
    if (offset == NULL)
        return 1;

    memcpy(a, b + *offset, 4);

    free(offset);

    return a[0];
}
EOF
clang -Weverything -fsanitize=memory -O0 main.c -o main-memsan
./main-memsan

For reference, valgrind's memcheck correctly catches the issue:

% clang -Weverything -O0 main.c -o main-plain && valgrind -q ./main-plain
==11366== Use of uninitialised value of size 8
==11366==    at 0x4005AD: main (in /tmp/main-plain)
==11366== 

Perhaps more importantly, if instead of calling the libc's memcpy() I use a re-implementation of it, then MSan also catches the issue:

#include <stdlib.h>

static void my_memcpy(char *dst, const char *src, size_t len) {
    for (size_t i = 0; i < len; i++)
        dst[i] = src[i];
}

static char a[16];
static const char b[16];

int main(void)
{
    size_t *offset = malloc(sizeof(size_t));
    if (offset == NULL)
        return 1;

    my_memcpy(a, b + *offset, 4);

    free(offset);

    return a[0];
}
% clang -Weverything -O0 -fsanitize=memory -g manual.c -o manual && ./manual
==29479==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x486cf5 in my_memcpy /tmp/manual.c:5:18
    #1 0x486a01 in main /tmp/manual.c:17:5
    #2 0x7f022419e83f in __libc_start_main /build/glibc-e6zv40/glibc-2.23/csu/../csu/libc-start.c:291
    #3 0x419418 in _start (/tmp/manual+0x419418)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /tmp/manual.c:5:18 in my_memcpy
Exiting

So apparently such issues are meant to be found by MSan, but the interceptor function __msan_memcpy() somehow misses it. (And again, that may be true of other interceptor functions, I didn't check.)

@vitalybuka
Copy link
Contributor

msan interceptors check memory pointed to, but do not check pointers or any other arguments as values.
It's known issue, but it's unlikely we will fix this in interceptors soon.

However I guess WIP "msan-eager-checks" may help to detect that before call (when we commit all related patches)

+@eugenis

daxtens pushed a commit to daxtens/mbedtls that referenced this issue May 24, 2021
The tests are supposed to be failing now (in all.sh component
test_memsan_constant_flow), but they don't as apparently MemSan doesn't
complain when the src argument of memcpy() is uninitialized, see
google/sanitizers#1296

The next commit will add an option to test constant flow with valgrind, which
will hopefully correctly flag the current non-constant-flow implementation.

Signed-off-by: Manuel Pégourié-Gonnard <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants