[PATCH v2 0/1] MR11014: setupapi: Initialize destW in SetupCopyOEMInfA (ASan).

[Bernhard Übelacker (＠bernhardu)]

Otherwise the call to WideCharToMultiByte does a lstrlenW,
which may overrun the uninitialized destW buffer.

This showed up in the [last ASan job](https://gitlab.winehq.org/bernhardu/wine/-/jobs/271183#L3584) as a result of some error before.

[Testbot run with this patch](https://testbot.winehq.org/JobDetails.pl?Key=163147)

<details>
<summary>ASan details</summary>

```
==setupapi_test.exe==752==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x102d403d8988 at pc 0x6ffffe8067f3 bp 0x7ffffe20e730 sp 0x7ffffe20e778
READ of size 522 at 0x102d403d8988 thread T0
    #0 0x6ffffe8067f2 in wcslen /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:7258:3
    #1 0x6fffff7b3cf1 in lstrlenW /builds/bernhardu/wine/build64/../include/winbase.h:2434:5
    #2 0x6fffff7b4845 in WideCharToMultiByte /builds/bernhardu/wine/build64/../dlls/kernelbase/locale.c:7417:30
    #3 0x6ffffb64c86a in SetupCopyOEMInfA /builds/bernhardu/wine/build64/../dlls/setupapi/devinst.c:5307:13
    #4 0x00014002fbe5 in test_copy_oem_inf /builds/bernhardu/wine/build64/../dlls/setupapi/tests/devinst.c:4731:11
    #5 0x000140001271 in func_devinst /builds/bernhardu/wine/build64/../dlls/setupapi/tests/devinst.c:5376:5
    #6 0x000140089cc2 in run_test /builds/bernhardu/wine/build64/../include/wine/test.h:780:5
    #7 0x0001400896cb in main /builds/bernhardu/wine/build64/../include/wine/test.h:900:12
    #8 0x00014008b96a in mainCRTStartup /builds/bernhardu/wine/build64/../dlls/msvcrt/crt_main.c:70:11
    #9 0x6fffffc37b64 in BaseThreadInitThunk (C:\windows\system32\kernel32.dll+0x178027b64)
    #10 0x6fffffdc0ab6 in RtlUserThreadStart (C:\windows\system32\ntdll.dll+0x170050ab6)
0x102d403d8988 is located 0 bytes after 520-byte region [0x102d403d8780,0x102d403d8988)
allocated by thread T0 here:
    #0 0x6ffffe80da21 in malloc /home/runner/work/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan/asan_malloc_win.cpp:87:3
    #1 0x6ffffb66e823 in MyMalloc /builds/bernhardu/wine/build64/../dlls/setupapi/misc.c:84:12
    #2 0x6ffffb64c723 in SetupCopyOEMInfA /builds/bernhardu/wine/build64/../dlls/setupapi/devinst.c:5295:27
    #3 0x00014002fbe5 in test_copy_oem_inf /builds/bernhardu/wine/build64/../dlls/setupapi/tests/devinst.c:4731:11
    #4 0x000140001271 in func_devinst /builds/bernhardu/wine/build64/../dlls/setupapi/tests/devinst.c:5376:5
    #5 0x000140089cc2 in run_test /builds/bernhardu/wine/build64/../include/wine/test.h:780:5
    #6 0x0001400896cb in main /builds/bernhardu/wine/build64/../include/wine/test.h:900:12
    #7 0x00014008b96a in mainCRTStartup /builds/bernhardu/wine/build64/../dlls/msvcrt/crt_main.c:70:11
    #8 0x6fffffc37b64 in BaseThreadInitThunk (C:\windows\system32\kernel32.dll+0x178027b64)
    #9 0x6fffffdc0ab6 in RtlUserThreadStart (C:\windows\system32\ntdll.dll+0x170050ab6)
SUMMARY: AddressSanitizer: heap-buffer-overflow /builds/bernhardu/wine/build64/../include/winbase.h:2434:5 in lstrlenW
```

</details>

--
  v2: setupapi: Only copy dest when it got written by SetupCopyOEMInfW (ASan).
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11014

[PATCH v2 1/1] setupapi: Only copy dest when it got written by SetupCopyOEMInfW (ASan).

[Bernhard Übelacker]

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

Otherwise the call to WideCharToMultiByte does a lstrlenW,
which may overrun the uninitialized destW buffer.
---
 dlls/setupapi/devinst.c       | 2 +-
 dlls/setupapi/tests/devinst.c | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/dlls/setupapi/devinst.c b/dlls/setupapi/devinst.c
index f6bfcc483b0..8f36ead4ef3 100644
--- a/dlls/setupapi/devinst.c
+++ b/dlls/setupapi/devinst.c
@@ -5300,7 +5300,7 @@ BOOL WINAPI SetupCopyOEMInfA( PCSTR source, PCSTR location,
 
     if (required_size) *required_size = size;
 
-    if (dest)
+    if (dest && (ret || GetLastError() == ERROR_FILE_EXISTS))
     {
         if (buffer_size >= size)
         {
diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c
index 98cec632219..d21a78b36ad 100644
--- a/dlls/setupapi/tests/devinst.c
+++ b/dlls/setupapi/tests/devinst.c
@@ -4654,6 +4654,13 @@ static void test_copy_oem_inf(struct testsign_context *ctx)
     ok(!ret, "Got %d.\n", ret);
     ok(GetLastError() == ERROR_FILE_NOT_FOUND, "Got error %#lx.\n", GetLastError());
 
+    /* try a relative nonexistent SourceInfFileName, with dest parameter */
+    memset(dest, 0xcc, sizeof(dest));
+    SetLastError(0xdeadbeef);
+    ret = SetupCopyOEMInfA("nonexistent", NULL, 0, SP_COPY_NOOVERWRITE, dest, sizeof(dest), NULL, NULL);
+    ok(!ret, "Got %d.\n", ret);
+    ok(GetLastError() == ERROR_FILE_NOT_FOUND, "Got error %#lx.\n", GetLastError());
+
     /* try an absolute nonexistent SourceInfFileName */
     GetCurrentDirectoryA(sizeof(path), path);
     strcat(path, "\\nonexistent");
-- 
GitLab

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

[PATCH v2 0/1] MR11014: setupapi: Only copy dest when it got written by SetupCopyOEMInfW (ASan).

[Bernhard Übelacker (＠bernhardu)]

v2: [Testbot run with this patch](https://testbot.winehq.org/JobDetails.pl?Key=163150)
- Remove initialisations and copy only depending on return values of SetupCopyOEMInfW.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11014#note_141617

[PATCH v2 0/1] MR11014: setupapi: Only copy dest when it got written by SetupCopyOEMInfW (ASan). - approved

[Elizabeth Figura (＠zfigura)]

This merge request was approved by Elizabeth Figura.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11014