[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>
From: Bernhard Übelacker bernhardu@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)
-
Bernhard Übelacker -
Bernhard Übelacker (＠bernhardu)