[PATCH v2 0/1] MR8071: ucrtbase/tests: Avoid reading behind buffer in test_swprintf (ASan).
The test `test_swprintf` creates an un-terminated buffer and calls `WideCharToMultiByte` without explicit srclen on it. This patch wraps the buffer into a struct providing a termination without modifying the test itself. <details> <summary>ASan output:</summary> ``` ================================================================= ==ucrtbase_test.exe==532==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffffe1ffac0 at pc 0x6ffffe845638 bp 0x7ffffe1ff030 sp 0x7ffffe1ff078 READ of size 24 at 0x7ffffe1ffac0 thread T0 #0 0x6ffffe845637 in wcslen C:/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/../sanitizer_common\sanitizer_common_interceptors.inc:7215:3 #1 0x6fffff7d1eff in lstrlenW .../wine/include/winbase.h:2574:5 #2 0x6fffff7d2a33 in WideCharToMultiByte .../wine/dlls/kernelbase/locale.c:7382:30 #3 0x000140016e6c in test_swprintf .../wine/dlls/ucrtbase/tests/printf.c:196:9 #4 0x000140015f3e in func_printf .../wine/dlls/ucrtbase/tests/printf.c:891:5 #5 0x00014002da0e in run_test .../wine/include/wine/test.h:765:5 #6 0x00014002d53a in main .../wine/include/wine/test.h:884:12 #7 0x00014002f005 in mainCRTStartup .../wine/dlls/msvcrt/crt_main.c:58:11 #8 0x6fffffc355be in BaseThreadInitThunk .../wine/dlls/kernel32/thread.c:61:24 #9 0x6fffffdbbcc2 in RtlUserThreadStart (C:\windows\system32\ntdll.dll+0x17004bcc2) Address 0x7ffffe1ffac0 is located in stack of thread T0 at offset 256 in frame #0 0x000140016b6f in test_swprintf .../wine/dlls/ucrtbase/tests/printf.c:176 This frame has 8 object(s): [32, 44) 'str_short' (line 177) [64, 80) 'str_justfit' (line 178) [96, 114) 'str_justfits' (line 179) [160, 182) 'str_muchlonger' (line 180) [224, 226) 'str_empty' (line 181) [240, 256) 'buffer' (line 184) [272, 280) 'narrow' (line 185) <== Memory access at offset 256 partially underflows this variable [304, 320) 'narrow_fmt' (line 185) HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork (longjmp, SEH and C++ exceptions *are* supported) SUMMARY: AddressSanitizer: stack-buffer-overflow .../wine/include/winbase.h:2574:5 in lstrlenW Shadow bytes around the buggy address: 0x7ffffe1ff800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x7ffffe1ff880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x7ffffe1ff900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x7ffffe1ff980: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 04 f2 f2 0x7ffffe1ffa00: 00 00 f2 f2 00 00 02 f2 f2 f2 f2 f2 00 00 06 f2 =>0x7ffffe1ffa80: f2 f2 f2 f2 02 f2 00 00[f2]f2 00 f2 f2 f2 00 00 0x7ffffe1ffb00: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 0x7ffffe1ffb80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x7ffffe1ffc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x7ffffe1ffc80: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 f8 f8 f8 f8 0x7ffffe1ffd00: f8 f8 f8 f8 f3 f3 f3 f3 00 00 00 00 00 00 00 00 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 ``` </details> -- v2: ucrtbase/tests: Avoid reading behind buffer in test_swprintf (ASan). https://gitlab.winehq.org/wine/wine/-/merge_requests/8071
From: Bernhard Übelacker <bernhardu(a)mailbox.org> --- dlls/ucrtbase/tests/printf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/ucrtbase/tests/printf.c b/dlls/ucrtbase/tests/printf.c index 75bd7830869..16dd4f3727b 100644 --- a/dlls/ucrtbase/tests/printf.c +++ b/dlls/ucrtbase/tests/printf.c @@ -193,7 +193,7 @@ static void test_swprintf (void) const int n = vswprintf_wrapper (_CRT_INTERNAL_PRINTF_LEGACY_VSPRINTF_NULL_TERMINATION, buffer, bufsiz, fmt); const int valid = n < 0 ? bufsiz : (n == bufsiz ? n : n+1); - WideCharToMultiByte (CP_ACP, 0, buffer, -1, narrow, sizeof(narrow), NULL, NULL); + WideCharToMultiByte (CP_ACP, 0, buffer, valid, narrow, sizeof(narrow), NULL, NULL); WideCharToMultiByte (CP_ACP, 0, fmt, -1, narrow_fmt, sizeof(narrow_fmt), NULL, NULL); ok (n == expect, "\"%s\": expected %d, returned %d\n", narrow_fmt, expect, n); @@ -208,7 +208,7 @@ static void test_swprintf (void) const int n = vswprintf_wrapper (_CRT_INTERNAL_PRINTF_STANDARD_SNPRINTF_BEHAVIOR, buffer, bufsiz, fmt); const int valid = n >= bufsiz ? bufsiz - 1 : n < 0 ? 0 : n; - WideCharToMultiByte (CP_ACP, 0, buffer, -1, narrow, sizeof(narrow), NULL, NULL); + WideCharToMultiByte (CP_ACP, 0, buffer, valid, narrow, sizeof(narrow), NULL, NULL); WideCharToMultiByte (CP_ACP, 0, fmt, -1, narrow_fmt, sizeof(narrow_fmt), NULL, NULL); ok (n == expect, "\"%s\": expected %d, returned %d\n", narrow_fmt, expect, n); @@ -225,7 +225,7 @@ static void test_swprintf (void) const int n = vswprintf_wrapper (0, buffer, bufsiz, fmt); const int valid = n < 0 ? bufsiz - 1 : n; - WideCharToMultiByte (CP_ACP, 0, buffer, -1, narrow, sizeof(narrow), NULL, NULL); + WideCharToMultiByte (CP_ACP, 0, buffer, valid, narrow, sizeof(narrow), NULL, NULL); WideCharToMultiByte (CP_ACP, 0, fmt, -1, narrow_fmt, sizeof(narrow_fmt), NULL, NULL); ok (n == expect, "\"%s\": expected %d, returned %d\n", narrow_fmt, expect, n); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8071
v2: - Instead of adding a termination, just providing the input length parameter to WideCharToMultiByte. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8071#note_103768
On Sun May 18 20:34:21 2025 +0000, Bernhard Übelacker wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/8071/diffs?diff_id=178813&start_sha=f40d5f5076e9f59f1480e8d50f305a34420f6890#9fb773a1833104fcdaa1e926cba1c05c5ad3e135_203_196) Thanks for having a look, I pushed a new version.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8071#note_103769
This merge request was approved by Piotr Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8071
participants (2)
-
Bernhard Übelacker -
Piotr Caban (＠piotr)