[PATCH 0/1] MR9529: user32: Avoid out-of-bounds read in DdeCreateDataHandle with offset (ASan).
Current implementation applies offset to the source buffer. But it looks like it has to be applied to the destination buffer. The bytes in the destination below the offset seem to get initialized with zero. [Testbot run with this patch](https://testbot.winehq.org/JobDetails.pl?Key=160951) <details> <summary>ASan report details</summary> ``` ================================================================= ==684==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000140292045 at pc 0x6ffffe8ccfac bp 0x7ffffe1feed0 sp 0x7ffffe1fef18 READ of size 5 at 0x000140292045 thread T0 #0 0x6ffffe8ccfab in memmove /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt\lib/asan/../sanitizer_common/sanitizer_common_interceptors_memintrinsics.inc:100:3 #1 0x6ffffa6dd6e4 in DdeCreateDataHandle .../wine/dlls/user32/dde_misc.c:1328 #2 0x000140061ffe in test_DdeCreateDataHandle .../wine/dlls/user32/tests/dde.c:1943 #3 0x00014005b743 in func_dde .../wine/dlls/user32/tests/dde.c:2764 #4 0x000140268dc2 in run_test .../wine/include/wine/test.h:780 #5 0x0001402687cb in main .../wine/include/wine/test.h:900 #6 0x00014026aa5a in mainCRTStartup .../wine/dlls/msvcrt/crt_main.c:62 #7 0x6ffffaf57a54 in BaseThreadInitThunk (C:\windows\system32\kernel32.dll+0x178027a54) #8 0x6ffffdcb06d6 in RtlUserThreadStart (C:\windows\system32\ntdll.dll+0x1700506d6) 0x000140292045 is located 27 bytes before global variable '"Expected non-NULL ptr\n"' defined in '.../wine/dlls/user32/tests/dde.c:1915' (0x000140292060) of size 23 '"Expected non-NULL ptr\n"' is ascii string 'Expected non-NULL ptr ' 0x000140292045 is located 0 bytes after global variable '"data"' defined in '.../wine/dlls/user32/tests/dde.c:1886' (0x000140292040) of size 5 '"data"' is ascii string 'data' SUMMARY: AddressSanitizer: global-buffer-overflow .../wine/dlls/user32/dde_misc.c:1328 in DdeCreateDataHandle ``` </details> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9529
From: Bernhard Übelacker <bernhardu(a)mailbox.org> --- dlls/user32/dde_misc.c | 3 ++- dlls/user32/tests/dde.c | 9 ++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/dlls/user32/dde_misc.c b/dlls/user32/dde_misc.c index b5ccf449692..8d55356bb84 100644 --- a/dlls/user32/dde_misc.c +++ b/dlls/user32/dde_misc.c @@ -1325,7 +1325,8 @@ HDDEDATA WINAPI DdeCreateDataHandle(DWORD idInst, LPBYTE pSrc, DWORD cb, DWORD c pByte = (LPBYTE)(pDdh + 1); if (pSrc) { - memcpy(pByte, pSrc + cbOff, cb); + if (cbOff) memset(pByte, 0, cbOff); + memcpy(pByte + cbOff, pSrc, cb); } GlobalUnlock(hMem); diff --git a/dlls/user32/tests/dde.c b/dlls/user32/tests/dde.c index c48d1fdfd87..a2fb64ead66 100644 --- a/dlls/user32/tests/dde.c +++ b/dlls/user32/tests/dde.c @@ -1850,6 +1850,7 @@ static void test_DdeCreateDataHandle(void) HSZ item; LPBYTE ptr; WCHAR item_str[] = {'i','t','e','m',0}; + const char* data1234 = "data1234"; dde_inst = 0; dde_inst2 = 0; @@ -1940,18 +1941,16 @@ static void test_DdeCreateDataHandle(void) /* cbOff is non-zero */ DdeGetLastError(dde_inst); - hdata = DdeCreateDataHandle(dde_inst, (LPBYTE)"data", 5, 2, item, CF_TEXT, 0); + hdata = DdeCreateDataHandle(dde_inst, (LPBYTE)(data1234 + 2), 5, 2, item, CF_TEXT, 0); err = DdeGetLastError(dde_inst); ok(hdata != NULL, "Expected non-NULL hdata\n"); ok(err == DMLERR_NO_ERROR, "Expected DMLERR_NO_ERROR, got %d\n", err); ptr = DdeAccessData(hdata, &size); ok(ptr != NULL, "Expected non-NULL ptr\n"); + ok(!memcmp(ptr, "\0\0ta1234", size), "Expected %s, got %s\n", + wine_dbgstr_an("\0\0ta1234", 7), wine_dbgstr_an((char*)ptr, 7)); ok(size == 7, "Expected 7, got %ld\n", size); - todo_wine - { - ok(ptr && !*ptr, "Expected 0, got %d\n", lstrlenA((LPSTR)ptr)); - } ret = DdeUnaccessData(hdata); ok(ret == TRUE, "Expected TRUE, got %d\n", ret); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9529
participants (1)
-
Bernhard Übelacker