[PATCH 0/1] MR10213: winmm/tests: Reserve additional memory in test_formats (ASan).

[Bernhard Übelacker]

Followup of fe239731.

In `test_formats` only `sizeof(WAVEFORMATEXTENSIBLE)` gets allocated, but in `WINMM_OpenDevice` it tries to read one addional byte in the "cbSize += 1" test.

CC: @giomasce 

[Test pattern page](https://test.winehq.org/data/patterns.html#winmm:wave) (This patch won't help here.)

[Testbot run with this patch](https://testbot.winehq.org/JobDetails.pl?Key=162097) (Shows also the other issues like the test pattern page.)

<details>

<summary>ASan details [gitlab ci run](https://gitlab.winehq.org/bernhardu/wine/-/jobs/232675#L2501)</summary>

```
==winmm_test.exe==320==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x0022fc78 at pc 0x784c4532 bp 0x04d3fbec sp 0x04d3f7bc
READ of size 41 at 0x0022fc78 thread T243
    #0 0x784c4531 in __asan_memcpy /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:65:3
    #1 0x78f3210e in WINMM_OpenDevice /builds/bernhardu/wine/build64/../dlls/winmm/waveform.c:1124:9
    #2 0x78f30aad in WOD_Open /builds/bernhardu/wine/build64/../dlls/winmm/waveform.c:1277:11
    #3 0x78f3638e in WINMM_DevicesMsgProc /builds/bernhardu/wine/build64/../dlls/winmm/waveform.c:2393:16
    #4 0x7b13b573 in WINPROC_wrapper (C:\windows\system32\user32.dll+0x1004b573)
    #5 0x7b13bdf3 in call_window_proc /builds/bernhardu/wine/build64/../dlls/user32/winproc.c:111:15
    #6 0x7b13beea in dispatch_win_proc_params /builds/bernhardu/wine/build64/../dlls/user32/winproc.c
    #7 0x7b13c807 in User32CallWinProc /builds/bernhardu/wine/build64/../dlls/user32/winproc.c:827:14
    #8 0x7be1dc86 in dispatch_user_callback /builds/bernhardu/wine/build64/../dlls/ntdll/exception.c:297:18
    #9 0x7be46a62 in KiUserCallbackDispatcher /builds/bernhardu/wine/build64/../dlls/ntdll/signal_i386.c:205:23
    #10 0x79d81d3b in NtUserPeekMessage (C:\windows\system32\win32u.dll+0x10011d3b)
    #11 0x78f361d6 in WINMM_DevicesThreadProc /builds/bernhardu/wine/build64/../dlls/winmm/waveform.c:2475:16
    #12 0x784d6f21 in asan_thread_start(void*) /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_win.cpp:147:14
    #13 0x7be4678a in call_thread_func_wrapper (C:\windows\system32\ntdll.dll+0x7bc4678a)
    #14 0x7be470fa in call_thread_func /builds/bernhardu/wine/build64/../dlls/ntdll/signal_i386.c:503:9
Address 0x0022fc78 is located in stack of thread T0 at offset 56 in frame
    #0 0x0043676f in test_formats /builds/bernhardu/wine/build64/../dlls/winmm/tests/wave.c:2434
  This frame has 1 object(s):
    [16, 56) 'fmt' (line 2455) <== Memory access at offset 56 overflows this variable
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 /builds/bernhardu/wine/build64/../dlls/winmm/waveform.c:1124:9 in WINMM_OpenDevice
```

</details>

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10213

[PATCH 1/1] winmm/tests: Reserve additional memory in test_formats (ASan).

[Bernhard Übelacker]

From: Bernhard Übelacker <[e-mail]>

Followup of fe239731.
---
 dlls/winmm/tests/wave.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/dlls/winmm/tests/wave.c b/dlls/winmm/tests/wave.c
index b23209541da..ca609a146d9 100644
--- a/dlls/winmm/tests/wave.c
+++ b/dlls/winmm/tests/wave.c
@@ -2452,12 +2452,15 @@ static void test_formats(void)
     for (i = 0; i < wave_format_count; ++i)
     {
         const char *additional_context = wave_formats[i].additional_context;
-        WAVEFORMATEXTENSIBLE fmt = wave_formats[i].format;
+        /* The test "cbSize += 1" needs to reserve additional memory. */
+        char buf[sizeof(WAVEFORMATEXTENSIBLE) + 1];
+        WAVEFORMATEXTENSIBLE *fmt = (WAVEFORMATEXTENSIBLE*)&buf;
+        *fmt = wave_formats[i].format;
 
         winetest_push_context("test %u%s%s", i, additional_context ? ", " : "",
                 additional_context ? additional_context : "");
-        push_format_context(&fmt);
-        test_format(&fmt);
+        push_format_context(fmt);
+        test_format(fmt);
         winetest_pop_context();
         winetest_pop_context();
     }
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/10213

[PATCH 0/1] MR10213: winmm/tests: Reserve additional memory in test_formats (ASan). - approved

[Giovanni Mascellani (＠giomasce)]

This merge request was approved by Giovanni Mascellani.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10213

[Giovanni Mascellani (＠giomasce)]

Thanks. In principle the same problem happens in in `dlls/mmdevapi/tests/render.c` and in `dlls/mmdevapi/tests/capture.c`, but since you didn't catch it I suppose mmdevapi is just more careful with not accessing bytes it doesn't need anyway.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10213#note_130916

[PATCH 0/1] MR10213: winmm/tests: Reserve additional memory in test_formats (ASan). - approved

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10213