[PATCH 0/1] MR9551: shell32: Avoid double-free in enumerate_strings when cur is zero (ASan).
If this realloc is reached and cur is zero, the realloc is called with a size of zero, which behaves like a free. And returns NULL, because of which the `fail:` label is reached, where the memory, strs is still pointing to, is freed the second time. I did not notice any difference in the tests when `enumerate_strings` is left via the `fail:` label or via the return. I am assuming the case when cur is zero is not intended to reach the `fail:` label. [This is a testbot run with this patch](https://testbot.winehq.org/JobDetails.pl?Key=161003) [ASan details from gitlab](https://gitlab.winehq.org/bernhardu/wine/-/jobs/210107#L2772) <details> <summary>ASan details</summary> ``` ================================================================= ==392==ERROR: AddressSanitizer: attempting double-free on 0x7fa7ca514900 in thread T0: #0 0x6ffffe8cd721 in free /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt\lib/asan/asan_malloc_win.cpp:71:3 #1 0x6ffffba8fa24 in enumerate_strings .../wine/dlls/shell32/autocomplete.c:245 #2 0x6ffffba8ea26 in autocomplete_text .../wine/dlls/shell32/autocomplete.c:637 #3 0x6ffffba8cf86 in ACEditSubclassProc .../wine/dlls/shell32/autocomplete.c:796 #4 0x6ffffd4a8bb0 in WINPROC_wrapper .../wine/dlls/user32/winproc.c:86 #5 0x6ffffd4a775c in call_window_proc .../wine/dlls/user32/winproc.c:111 #6 0x6ffffd4a78be in dispatch_win_proc_params .../wine/dlls/user32/winproc.c #7 0x6ffffd492022 in dispatch_send_message .../wine/dlls/user32/message.c:568 #8 0x6ffffd491fa6 in SendMessageW .../wine/dlls/user32/message.c:587 #9 0x0001400107b9 in test_string_enumerator .../wine/dlls/shell32/tests/autocomplete.c:731 #10 0x00014000cb22 in test_custom_source .../wine/dlls/shell32/tests/autocomplete.c:904 #11 0x00014000a6bf in func_autocomplete .../wine/dlls/shell32/tests/autocomplete.c:938 #12 0x0001400d6c62 in run_test .../wine/include/wine/test.h:780 #13 0x0001400d666b in main .../wine/include/wine/test.h:900 #14 0x0001400d88fa in mainCRTStartup .../wine/dlls/msvcrt/crt_main.c:62 #15 0x6fffffc67a54 in BaseThreadInitThunk (C:\windows\system32\kernel32.dll+0x178027a54) #16 0x6fffffde06e6 in signal_start_thread (C:\windows\system32\ntdll.dll+0x1700506e6) 0x7fa7ca514900 is located 0 bytes inside of 8192-byte region [0x7fa7ca514900,0x7fa7ca516900) freed by thread T0 here: #0 0x6ffffe8cda76 in realloc /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt\lib/asan/asan_malloc_win.cpp:110:3 #1 0x6ffffba8f964 in enumerate_strings .../wine/dlls/shell32/autocomplete.c:231 #2 0x6ffffba8ea26 in autocomplete_text .../wine/dlls/shell32/autocomplete.c:637 #3 0x6ffffba8cf86 in ACEditSubclassProc .../wine/dlls/shell32/autocomplete.c:796 #4 0x6ffffd4a8bb0 in WINPROC_wrapper .../wine/dlls/user32/winproc.c:86 #5 0x6ffffd4a775c in call_window_proc .../wine/dlls/user32/winproc.c:111 #6 0x6ffffd4a78be in dispatch_win_proc_params .../wine/dlls/user32/winproc.c #7 0x6ffffd492022 in dispatch_send_message .../wine/dlls/user32/message.c:568 #8 0x6ffffd491fa6 in SendMessageW .../wine/dlls/user32/message.c:587 #9 0x0001400107b9 in test_string_enumerator .../wine/dlls/shell32/tests/autocomplete.c:731 #10 0x00014000cb22 in test_custom_source .../wine/dlls/shell32/tests/autocomplete.c:904 #11 0x00014000a6bf in func_autocomplete .../wine/dlls/shell32/tests/autocomplete.c:938 #12 0x0001400d6c62 in run_test .../wine/include/wine/test.h:780 #13 0x0001400d666b in main .../wine/include/wine/test.h:900 #14 0x0001400d88fa in mainCRTStartup .../wine/dlls/msvcrt/crt_main.c:62 #15 0x6fffffc67a54 in BaseThreadInitThunk (C:\windows\system32\kernel32.dll+0x178027a54) #16 0x6fffffde06e6 in signal_start_thread (C:\windows\system32\ntdll.dll+0x1700506e6) previously allocated by thread T0 here: #0 0x6ffffe8cda76 in realloc /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt\lib/asan/asan_malloc_win.cpp:110:3 #1 0x6ffffba8f8ee in enumerate_strings .../wine/dlls/shell32/autocomplete.c:212 #2 0x6ffffba8ea26 in autocomplete_text .../wine/dlls/shell32/autocomplete.c:637 #3 0x6ffffba8cf86 in ACEditSubclassProc .../wine/dlls/shell32/autocomplete.c:796 #4 0x6ffffd4a8bb0 in WINPROC_wrapper .../wine/dlls/user32/winproc.c:86 #5 0x6ffffd4a775c in call_window_proc .../wine/dlls/user32/winproc.c:111 #6 0x6ffffd4a78be in dispatch_win_proc_params .../wine/dlls/user32/winproc.c #7 0x6ffffd492022 in dispatch_send_message .../wine/dlls/user32/message.c:568 #8 0x6ffffd491fa6 in SendMessageW .../wine/dlls/user32/message.c:587 #9 0x0001400107b9 in test_string_enumerator .../wine/dlls/shell32/tests/autocomplete.c:731 #10 0x00014000cb22 in test_custom_source .../wine/dlls/shell32/tests/autocomplete.c:904 #11 0x00014000a6bf in func_autocomplete .../wine/dlls/shell32/tests/autocomplete.c:938 #12 0x0001400d6c62 in run_test .../wine/include/wine/test.h:780 #13 0x0001400d666b in main .../wine/include/wine/test.h:900 #14 0x0001400d88fa in mainCRTStartup .../wine/dlls/msvcrt/crt_main.c:62 #15 0x6fffffc67a54 in BaseThreadInitThunk (C:\windows\system32\kernel32.dll+0x178027a54) #16 0x6fffffde06e6 in signal_start_thread (C:\windows\system32\ntdll.dll+0x1700506e6) ``` </details> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9551
From: Bernhard Übelacker <bernhardu(a)mailbox.org> If this realloc is reached and cur is zero, the realloc is called with a size of zero, which behaves like a free. And returns NULL, because of which the fail: label is reached, where the memory, strs is still pointing to, is freed the second time. --- dlls/shell32/autocomplete.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c index d027722cd50..e39e3662d48 100644 --- a/dlls/shell32/autocomplete.c +++ b/dlls/shell32/autocomplete.c @@ -228,7 +228,7 @@ static void enumerate_strings(IAutoCompleteImpl *ac, enum prefix_filtering pfx_f } while (str_read); /* Allocate even if there were zero strings enumerated, to mark it non-NULL */ - if ((tmp = realloc(strs, cur * sizeof(*strs)))) + if ((tmp = realloc(strs, max(cur, 1) * sizeof(*strs)))) { strs = tmp; if (cur > 0) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9551
participants (1)
-
Bernhard Übelacker