[PATCH 0/1] MR7544: ieframe: Enter reallocation path one position earlier (ASan).
Trying to browse through the debugger.chm from an old windbg version with hh.exe, I encountered below ASan message. The reallocation path was not entered when `This->travellog.size < This->travellog.position+1`, with e.g. size == 4 and position == 3. Unfortunately the increment of position takes place a few lines later, therefore writing at index 4 of array with size 4. ``` ================================================================= ==hh.exe==316==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7e99833c9a80 at pc 0x6ffffe8490da bp 0x7ffffe1fea70 sp 0x7ffffe1feab0 WRITE of size 16 at 0x7e99833c9a80 thread T0 #0 0x6ffffe8490d9 in __asan_memset /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt\lib/asan/asan_interceptors_memintrinsics.cpp:67:3 #1 0x6ffffa7cb968 in update_travellog /home/bernhard/data/entwicklung/2025/wine\wine/dlls/ieframe/dochost.c:471:59 #2 0x6ffffa7caf09 in ClOleCommandTarget_Exec /home/bernhard/data/entwicklung/2025/wine\wine/dlls/ieframe/dochost.c:780:13 #3 0x6ffff929b38a in IOleCommandTarget_Exec /home/bernhard/data/entwicklung/2025/wine/wine-build/llvm-newwow64-asan-pe/obj\include\docobj.h:848:12 #4 0x6ffff929ad8c in notify_travellog_update /home/bernhard/data/entwicklung/2025/wine\wine/dlls/mshtml/persist.c:71:9 #5 0x6ffff929a82b in set_current_mon /home/bernhard/data/entwicklung/2025/wine\wine/dlls/mshtml/persist.c:108:17 #6 0x6ffff923abb7 in navigate_proc /home/bernhard/data/entwicklung/2025/wine\wine/dlls/mshtml/navigate.c:2155:9 #7 0x6ffff92d43df in hidden_proc /home/bernhard/data/entwicklung/2025/wine\wine/dlls/mshtml/task.c:398:17 #8 0x6ffffd3f3c8e in WINPROC_wrapper /home/bernhard/data/entwicklung/2025/wine\wine/dlls/user32/winproc.c:86:12 #9 0x6ffffd3f28c8 in call_window_proc /home/bernhard/data/entwicklung/2025/wine\wine/dlls/user32/winproc.c:111:15 #10 0x6ffffd3f2ab2 in dispatch_win_proc_params /home/bernhard/data/entwicklung/2025/wine\wine/dlls/user32/winproc.c #11 0x6ffffd3df4fb in dispatch_message /home/bernhard/data/entwicklung/2025/wine\wine/dlls/user32/message.c:804:14 #12 0x6ffffd3df5a1 in DispatchMessageW /home/bernhard/data/entwicklung/2025/wine\wine/dlls/user32/message.c:890:16 #13 0x6ffffdc63d8e in doWinMain /home/bernhard/data/entwicklung/2025/wine\wine/dlls/hhctrl.ocx/hhctrl.c:580:9 #14 0x000140001036 in WinMain /home/bernhard/data/entwicklung/2025/wine\wine/programs/hh/main.c:34:12 #15 0x00014000118f in main /home/bernhard/data/entwicklung/2025/wine\wine/dlls/msvcrt/crt_winmain.c:53:12 #16 0x0001400010b5 in mainCRTStartup /home/bernhard/data/entwicklung/2025/wine\wine/dlls/msvcrt/crt_main.c:58:11 #17 0x6fffffc3565e in BaseThreadInitThunk /home/bernhard/data/entwicklung/2025/wine\wine/dlls/kernel32/thread.c:61:24 #18 0x6fffffdbba1a in RtlUserThreadStart (C:\windows\system32\ntdll.dll+0x17004ba1a) 0x7e99833c9a80 is located 0 bytes after 64-byte region [0x7e99833c9a40,0x7e99833c9a80) allocated by thread T0 here: #0 0x6ffffe84a4a1 in malloc /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt\lib/asan/asan_malloc_win.cpp:80:3 #1 0x6ffffa7cb68b in update_travellog /home/bernhard/data/entwicklung/2025/wine\wine/dlls/ieframe/dochost.c:437:31 #2 0x6ffffa7caf09 in ClOleCommandTarget_Exec /home/bernhard/data/entwicklung/2025/wine\wine/dlls/ieframe/dochost.c:780:13 #3 0x6ffff929b38a in IOleCommandTarget_Exec /home/bernhard/data/entwicklung/2025/wine/wine-build/llvm-newwow64-asan-pe/obj\include\docobj.h:848:12 #4 0x6ffff929ad8c in notify_travellog_update /home/bernhard/data/entwicklung/2025/wine\wine/dlls/mshtml/persist.c:71:9 #5 0x6ffff929a82b in set_current_mon /home/bernhard/data/entwicklung/2025/wine\wine/dlls/mshtml/persist.c:108:17 #6 0x6ffff923abb7 in navigate_proc /home/bernhard/data/entwicklung/2025/wine\wine/dlls/mshtml/navigate.c:2155:9 #7 0x6ffff92d43df in hidden_proc /home/bernhard/data/entwicklung/2025/wine\wine/dlls/mshtml/task.c:398:17 #8 0x6ffffd3f3c8e in WINPROC_wrapper /home/bernhard/data/entwicklung/2025/wine\wine/dlls/user32/winproc.c:86:12 #9 0x6ffffd3f28c8 in call_window_proc /home/bernhard/data/entwicklung/2025/wine\wine/dlls/user32/winproc.c:111:15 #10 0x6ffffd3f2ab2 in dispatch_win_proc_params /home/bernhard/data/entwicklung/2025/wine\wine/dlls/user32/winproc.c #11 0x6ffffd3df4fb in dispatch_message /home/bernhard/data/entwicklung/2025/wine\wine/dlls/user32/message.c:804:14 #12 0x6ffffd3df5a1 in DispatchMessageW /home/bernhard/data/entwicklung/2025/wine\wine/dlls/user32/message.c:890:16 #13 0x6ffffdc63d8e in doWinMain /home/bernhard/data/entwicklung/2025/wine\wine/dlls/hhctrl.ocx/hhctrl.c:580:9 #14 0x000140001036 in WinMain /home/bernhard/data/entwicklung/2025/wine\wine/programs/hh/main.c:34:12 #15 0x00014000118f in main /home/bernhard/data/entwicklung/2025/wine\wine/dlls/msvcrt/crt_winmain.c:53:12 #16 0x0001400010b5 in mainCRTStartup /home/bernhard/data/entwicklung/2025/wine\wine/dlls/msvcrt/crt_main.c:58:11 #17 0x6fffffc3565e in BaseThreadInitThunk /home/bernhard/data/entwicklung/2025/wine\wine/dlls/kernel32/thread.c:61:24 #18 0x6fffffdbba1a in RtlUserThreadStart (C:\windows\system32\ntdll.dll+0x17004ba1a) SUMMARY: AddressSanitizer: heap-buffer-overflow /home/bernhard/data/entwicklung/2025/wine\wine/dlls/ieframe/dochost.c:471:59 in update_travellog Shadow bytes around the buggy address: 0x7e99833c9800: fd fd fd fd fd fd fd fa fa fa fa fa fd fd fd fd 0x7e99833c9880: fd fd fd fa fa fa fa fa fd fd fd fd fd fd fd fd 0x7e99833c9900: fa fa fa fa 00 00 00 00 00 00 00 fa fa fa fa fa 0x7e99833c9980: 00 00 00 00 00 00 00 fa fa fa fa fa fd fd fd fd 0x7e99833c9a00: fd fd fd fd fa fa fa fa 00 00 00 00 00 00 00 00 =>0x7e99833c9a80:[fa]fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa 0x7e99833c9b00: 00 00 00 00 00 00 00 fa fa fa fa fa fd fd fd fd 0x7e99833c9b80: fd fd fd fa fa fa fa fa 00 00 00 00 00 00 00 fa 0x7e99833c9c00: fa fa fa fa 00 00 00 00 00 00 00 fa fa fa fa fa 0x7e99833c9c80: 00 00 00 00 00 00 00 fa fa fa fa fa fd fd fd fd 0x7e99833c9d00: fd fd fd fd fa fa fa fa fd fd fd fd fd fd fd fd Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7544
From: Bernhard Übelacker <bernhardu(a)mailbox.org> --- dlls/ieframe/dochost.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/ieframe/dochost.c b/dlls/ieframe/dochost.c index 9d943594614..823cd8aba2d 100644 --- a/dlls/ieframe/dochost.c +++ b/dlls/ieframe/dochost.c @@ -439,7 +439,7 @@ static void update_travellog(DocHost *This) return; This->travellog.size = 4; - }else if(This->travellog.size < This->travellog.position+1) { + }else if(This->travellog.size <= This->travellog.position+1) { travellog_entry_t *new_travellog; new_travellog = realloc(This->travellog.log, This->travellog.size * 2 * sizeof(*This->travellog.log)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7544
Jinoh Kang (@iamahuman) commented about dlls/ieframe/dochost.c:
new_entry->stream = get_travellog_stream(This);
if(This->travellog.loading_pos == -1) { This->travellog.position++;
We should assert here that position is still less than size after the increment. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7544#note_97425
Jinoh Kang (@iamahuman) commented about dlls/ieframe/dochost.c:
return;
This->travellog.size = 4; - }else if(This->travellog.size < This->travellog.position+1) { + }else if(This->travellog.size <= This->travellog.position+1) {
```suggestion:-0+0 }else if(This->travellog.size < This->travellog.position+2) { ``` This looks more intuitive because it makes it clear we want to append 2 slots max. (Integer overflow is probably not a concern, and it is already a possibility.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7544#note_97426
participants (2)
-
Bernhard Übelacker -
Jinoh Kang (＠iamahuman)