From 49560458426cf25b6a36bbf9bad35aa75c9f7aa7 Mon Sep 17 00:00:00 2001 From: Dan Kegel Date: Sat, 28 Jul 2012 09:43:43 -0700 Subject: [PATCH] msvcrt: memmove_s shouldn't zero its output buffer on error. --- dlls/msvcrt/heap.c | 33 ++++++--- dlls/msvcrt/msvcrt.spec | 2 +- dlls/msvcrt/tests/misc.c | 54 -------------- dlls/msvcrt/tests/string.c | 148 +++++++++++++++++++++++++++++++++++++ 4 files changed, 173 insertions(+), 64 deletions(-) diff --git a/dlls/msvcrt/heap.c b/dlls/msvcrt/heap.c index 962119e91d2..5eaf4a1c5eb 100644 --- a/dlls/msvcrt/heap.c +++ b/dlls/msvcrt/heap.c @@ -554,22 +554,37 @@ int CDECL memmove_s(void *dest, MSVCRT_size_t numberOfElements, const void *src, if(!count) return 0; - if(!dest || !src) { - if(dest) - memset(dest, 0, numberOfElements); + if (!MSVCRT_CHECK_PMT(dest != NULL)) return MSVCRT_EINVAL; + if (!MSVCRT_CHECK_PMT(src != NULL)) return MSVCRT_EINVAL; + if (!MSVCRT_CHECK_PMT_ERR( count <= numberOfElements, MSVCRT_ERANGE )) return MSVCRT_ERANGE; - *MSVCRT__errno() = MSVCRT_EINVAL; + memmove(dest, src, count); + return 0; +} + +/********************************************************************* + * memcpy_s (MSVCRT.@) + */ +int CDECL memcpy_s(void *dest, MSVCRT_size_t numberOfElements, const void *src, MSVCRT_size_t count) +{ + TRACE("(%p %lu %p %lu)\n", dest, numberOfElements, src, count); + + if(!count) + return 0; + + if (!MSVCRT_CHECK_PMT(dest != NULL)) return MSVCRT_EINVAL; + if (!MSVCRT_CHECK_PMT(src != NULL)) + { + memset(dest, 0, numberOfElements); return MSVCRT_EINVAL; } - - if(count > numberOfElements) { + if (!MSVCRT_CHECK_PMT_ERR( count <= numberOfElements, MSVCRT_ERANGE )) + { memset(dest, 0, numberOfElements); - - *MSVCRT__errno() = MSVCRT_ERANGE; return MSVCRT_ERANGE; } - memmove(dest, src, count); + memcpy(dest, src, count); return 0; } diff --git a/dlls/msvcrt/msvcrt.spec b/dlls/msvcrt/msvcrt.spec index c2e4f3ec4d7..03181c03c90 100644 --- a/dlls/msvcrt/msvcrt.spec +++ b/dlls/msvcrt/msvcrt.spec @@ -1339,7 +1339,7 @@ @ cdecl memchr(ptr long long) ntdll.memchr @ cdecl memcmp(ptr ptr long) ntdll.memcmp @ cdecl memcpy(ptr ptr long) ntdll.memcpy -@ cdecl memcpy_s(ptr long ptr long) memmove_s +@ cdecl memcpy_s(ptr long ptr long) @ cdecl memmove(ptr ptr long) ntdll.memmove @ cdecl memmove_s(ptr long ptr long) @ cdecl memset(ptr long long) ntdll.memset diff --git a/dlls/msvcrt/tests/misc.c b/dlls/msvcrt/tests/misc.c index 7135248fb47..a2bf0cb8345 100644 --- a/dlls/msvcrt/tests/misc.c +++ b/dlls/msvcrt/tests/misc.c @@ -23,7 +23,6 @@ #include "msvcrt.h" static int (__cdecl *prand_s)(unsigned int *); -static int (__cdecl *pmemcpy_s)(void *, MSVCRT_size_t, void*, MSVCRT_size_t); static int (__cdecl *pI10_OUTPUT)(long double, int, int, void*); static int (__cdecl *pstrerror_s)(char *, MSVCRT_size_t, int); static int (__cdecl *p_get_doserrno)(int *); @@ -36,7 +35,6 @@ static void init(void) HMODULE hmod = GetModuleHandleA("msvcrt.dll"); prand_s = (void *)GetProcAddress(hmod, "rand_s"); - pmemcpy_s = (void*)GetProcAddress(hmod, "memcpy_s"); pI10_OUTPUT = (void*)GetProcAddress(hmod, "$I10_OUTPUT"); pstrerror_s = (void *)GetProcAddress(hmod, "strerror_s"); p_get_doserrno = (void *)GetProcAddress(hmod, "_get_doserrno"); @@ -65,57 +63,6 @@ static void test_rand_s(void) ok(ret == 0, "Expected rand_s to return 0, got %d\n", ret); } -static void test_memcpy_s(void) -{ - static char data[] = "data\0to\0be\0copied"; - static char dest[32]; - int ret; - - if(!pmemcpy_s) - { - win_skip("memcpy_s is not available\n"); - return; - } - - errno = 0xdeadbeef; - ret = pmemcpy_s(NULL, 0, NULL, 0); - ok(ret == 0, "ret = %x\n", ret); - ok(errno == 0xdeadbeef, "errno = %x\n", errno); - - errno = 0xdeadbeef; - dest[0] = 'x'; - ret = pmemcpy_s(dest, 10, NULL, 0); - ok(ret == 0, "ret = %x\n", ret); - ok(errno == 0xdeadbeef, "errno = %x\n", errno); - ok(dest[0] == 'x', "dest[0] != \'x\'\n"); - - errno = 0xdeadbeef; - ret = pmemcpy_s(NULL, 10, data, 10); - ok(ret == EINVAL, "ret = %x\n", ret); - ok(errno == EINVAL, "errno = %x\n", errno); - - errno = 0xdeadbeef; - dest[7] = 'x'; - ret = pmemcpy_s(dest, 10, data, 5); - ok(ret == 0, "ret = %x\n", ret); - ok(errno == 0xdeadbeef, "errno = %x\n", errno); - ok(memcmp(dest, data, 10), "All data copied\n"); - ok(!memcmp(dest, data, 5), "First five bytes are different\n"); - - errno = 0xdeadbeef; - ret = pmemcpy_s(data, 10, data, 10); - ok(ret == 0, "ret = %x\n", ret); - ok(errno == 0xdeadbeef, "errno = %x\n", errno); - ok(!memcmp(dest, data, 5), "data was destroyed during overwriting\n"); - - errno = 0xdeadbeef; - dest[0] = 'x'; - ret = pmemcpy_s(dest, 5, data, 10); - ok(ret == ERANGE, "ret = %x\n", ret); - ok(errno == ERANGE, "errno = %x\n", errno); - ok(dest[0] == '\0', "dest[0] != \'\\0\'\n"); -} - typedef struct _I10_OUTPUT_data { short pos; char sign; @@ -367,7 +314,6 @@ START_TEST(misc) init(); test_rand_s(); - test_memcpy_s(); test_I10_OUTPUT(); test_strerror_s(); test__get_doserrno(); diff --git a/dlls/msvcrt/tests/string.c b/dlls/msvcrt/tests/string.c index 6f369287249..909077fdffc 100644 --- a/dlls/msvcrt/tests/string.c +++ b/dlls/msvcrt/tests/string.c @@ -55,6 +55,8 @@ static void __cdecl test_invalid_parameter_handler(const wchar_t *expression, #define expect_bin(buf, value, len) { ok(memcmp((buf), value, len) == 0, "Binary buffer mismatch - expected %s, got %s\n", buf_to_string((unsigned char *)value, len, 1), buf_to_string((buf), len, 0)); } static void* (__cdecl *pmemcpy)(void *, const void *, size_t n); +static int (__cdecl *p_memcpy_s)(void *, size_t, const void *, size_t); +static int (__cdecl *p_memmove_s)(void *, size_t, const void *, size_t); static int* (__cdecl *pmemcmp)(void *, const void *, size_t n); static int (__cdecl *pstrcpy_s)(char *dst, size_t len, const char *src); static int (__cdecl *pstrcat_s)(char *dst, size_t len, const char *src); @@ -497,6 +499,148 @@ static void test_strcpy_s(void) ok(ret == EINVAL, "Copying a big string a NULL dest returned %d, expected EINVAL\n", ret); } +#define NUMELMS(array) (sizeof(array)/sizeof((array)[0])) + +#define okchars(dst, b0, b1, b2, b3, b4, b5, b6, b7) \ + ok(dst[0] == b0 && dst[1] == b1 && dst[2] == b2 && dst[3] == b3 && \ + dst[4] == b4 && dst[5] == b5 && dst[6] == b6 && dst[7] == b7, \ + "Bad result: 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x 0x%02x\n",\ + dst[0], dst[1], dst[2], dst[3], dst[4], dst[5], dst[6], dst[7]) + +static void test_memcpy_s(void) +{ + static char dest[8]; + static const char tiny[] = {'T',0,'I','N','Y',0}; + static const char big[] = {'a','t','o','o','l','o','n','g','s','t','r','i','n','g',0}; + int ret; + if (!p_memcpy_s) { + skip("memcpy_s not found\n"); + return; + } + + if (p_set_invalid_parameter_handler) + ok(p_set_invalid_parameter_handler(test_invalid_parameter_handler) == NULL, + "Invalid parameter handler was already set\n"); + + /* Normal */ + memset(dest, 'X', sizeof(dest)); + ret = p_memcpy_s(dest, NUMELMS(dest), tiny, NUMELMS(tiny)); + ok(ret == 0, "Copying a buffer into a big enough destination returned %d, expected 0\n", ret); + okchars(dest, tiny[0], tiny[1], tiny[2], tiny[3], tiny[4], tiny[5], 'X', 'X'); + + /* Vary source size */ + errno = 0xdeadbeef; + memset(dest, 'X', sizeof(dest)); + ret = p_memcpy_s(dest, NUMELMS(dest), big, NUMELMS(big)); + ok(ret == ERANGE, "Copying a big buffer to a small destination returned %d, expected ERANGE\n", ret); + ok(errno == ERANGE, "errno is %d, expected ERANGE\n", errno); + okchars(dest, 0, 0, 0, 0, 0, 0, 0, 0); + + /* Replace source with NULL */ + errno = 0xdeadbeef; + memset(dest, 'X', sizeof(dest)); + ret = p_memcpy_s(dest, NUMELMS(dest), NULL, NUMELMS(tiny)); + ok(ret == EINVAL, "Copying a NULL source buffer returned %d, expected EINVAL\n", ret); + ok(errno == EINVAL, "errno is %d, expected EINVAL\n", errno); + okchars(dest, 0, 0, 0, 0, 0, 0, 0, 0); + + /* Vary dest size */ + errno = 0xdeadbeef; + memset(dest, 'X', sizeof(dest)); + ret = p_memcpy_s(dest, 0, tiny, NUMELMS(tiny)); + ok(ret == ERANGE, "Copying into a destination of size 0 returned %d, expected ERANGE\n", ret); + ok(errno == ERANGE, "errno is %d, expected ERANGE\n", errno); + okchars(dest, 'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X'); + + /* Replace dest with NULL */ + errno = 0xdeadbeef; + ret = p_memcpy_s(NULL, NUMELMS(dest), tiny, NUMELMS(tiny)); + ok(ret == EINVAL, "Copying a tiny buffer to a big NULL destination returned %d, expected EINVAL\n", ret); + ok(errno == EINVAL, "errno is %d, expected EINVAL\n", errno); + + /* Combinations */ + errno = 0xdeadbeef; + memset(dest, 'X', sizeof(dest)); + ret = p_memcpy_s(dest, 0, NULL, NUMELMS(tiny)); + ok(ret == EINVAL, "Copying a NULL buffer into a destination of size 0 returned %d, expected EINVAL\n", ret); + ok(errno == EINVAL, "errno is %d, expected EINVAL\n", errno); + okchars(dest, 'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X'); + + if (p_set_invalid_parameter_handler) + ok(p_set_invalid_parameter_handler(NULL) == test_invalid_parameter_handler, + "Cannot reset invalid parameter handler\n"); +} + +static void test_memmove_s(void) +{ + static char dest[8]; + static const char tiny[] = {'T',0,'I','N','Y',0}; + static const char big[] = {'a','t','o','o','l','o','n','g','s','t','r','i','n','g',0}; + int ret; + if (!p_memmove_s) { + skip("memmove_s not found\n"); + return; + } + + if (p_set_invalid_parameter_handler) + ok(p_set_invalid_parameter_handler(test_invalid_parameter_handler) == NULL, + "Invalid parameter handler was already set\n"); + + /* Normal */ + memset(dest, 'X', sizeof(dest)); + ret = p_memmove_s(dest, NUMELMS(dest), tiny, NUMELMS(tiny)); + ok(ret == 0, "Moving a buffer into a big enough destination returned %d, expected 0\n", ret); + okchars(dest, tiny[0], tiny[1], tiny[2], tiny[3], tiny[4], tiny[5], 'X', 'X'); + + /* Overlapping */ + memcpy(dest, big, sizeof(dest)); + ret = p_memmove_s(dest+1, NUMELMS(dest)-1, dest, NUMELMS(dest)-1); + ok(ret == 0, "Moving a buffer up one char returned %d, expected 0\n", ret); + okchars(dest, big[0], big[0], big[1], big[2], big[3], big[4], big[5], big[6]); + + /* Vary source size */ + errno = 0xdeadbeef; + memset(dest, 'X', sizeof(dest)); + ret = p_memmove_s(dest, NUMELMS(dest), big, NUMELMS(big)); + ok(ret == ERANGE, "Moving a big buffer to a small destination returned %d, expected ERANGE\n", ret); + ok(errno == ERANGE, "errno is %d, expected ERANGE\n", errno); + okchars(dest, 'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X'); + + /* Replace source with NULL */ + errno = 0xdeadbeef; + memset(dest, 'X', sizeof(dest)); + ret = p_memmove_s(dest, NUMELMS(dest), NULL, NUMELMS(tiny)); + ok(ret == EINVAL, "Moving a NULL source buffer returned %d, expected EINVAL\n", ret); + ok(errno == EINVAL, "errno is %d, expected EINVAL\n", errno); + okchars(dest, 'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X'); + + /* Vary dest size */ + errno = 0xdeadbeef; + memset(dest, 'X', sizeof(dest)); + ret = p_memmove_s(dest, 0, tiny, NUMELMS(tiny)); + ok(ret == ERANGE, "Moving into a destination of size 0 returned %d, expected ERANGE\n", ret); + ok(errno == ERANGE, "errno is %d, expected ERANGE\n", errno); + okchars(dest, 'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X'); + + /* Replace dest with NULL */ + errno = 0xdeadbeef; + ret = p_memmove_s(NULL, NUMELMS(dest), tiny, NUMELMS(tiny)); + ok(ret == EINVAL, "Moving a tiny buffer to a big NULL destination returned %d, expected EINVAL\n", ret); + ok(errno == EINVAL, "errno is %d, expected EINVAL\n", errno); + + /* Combinations */ + errno = 0xdeadbeef; + memset(dest, 'X', sizeof(dest)); + ret = p_memmove_s(dest, 0, NULL, NUMELMS(tiny)); + ok(ret == EINVAL, "Moving a NULL buffer into a destination of size 0 returned %d, expected EINVAL\n", ret); + ok(errno == EINVAL, "errno is %d, expected EINVAL\n", errno); + okchars(dest, 'X', 'X', 'X', 'X', 'X', 'X', 'X', 'X'); + + if (p_set_invalid_parameter_handler) + ok(p_set_invalid_parameter_handler(NULL) == test_invalid_parameter_handler, + "Cannot reset invalid parameter handler\n"); +} + static void test_strcat_s(void) { char dest[8]; @@ -2129,6 +2273,8 @@ START_TEST(string) hMsvcrt = GetModuleHandleA("msvcrtd.dll"); ok(hMsvcrt != 0, "GetModuleHandleA failed\n"); SET(pmemcpy,"memcpy"); + p_memcpy_s = (void*)GetProcAddress( hMsvcrt, "memcpy_s" ); + p_memmove_s = (void*)GetProcAddress( hMsvcrt, "memmove_s" ); SET(pmemcmp,"memcmp"); SET(p_mbctype,"_mbctype"); SET(p__mb_cur_max,"__mb_cur_max"); @@ -2176,6 +2322,8 @@ START_TEST(string) /* test _strdup */ test_strdup(); test_strcpy_s(); + test_memcpy_s(); + test_memmove_s(); test_strcat_s(); test__mbsnbcpy_s(); test_mbcjisjms();