[PATCH 0/1] MR7798: spoolss: Avoid buffer-overflow when setting numentries. (ASan)

[Bernhard Übelacker]

BuildOtherNamesFromMachineName writes sizeof(LPVOID) bytes but the test reserved just sizeof(DWORD), which makes a difference at amd64 and gets detected by ASan.

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

```
=================================================================
==spoolss_test.exe==700==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffffe1ffba0 at pc 0x6ffffdc53e38 bp 0x7ffffe1ffa40 sp 0x7ffffe1ffa88
WRITE of size 8 at 0x7ffffe1ffba0 thread T0
    #0 0x6ffffdc53e37 in BuildOtherNamesFromMachineName .../wine/dlls/spoolss/spoolss_main.c:99:11
    #1 0x00014000170e in test_BuildOtherNamesFromMachineName .../wine/dlls/spoolss/tests/spoolss.c:131:11
    #2 0x000140001030 in func_spoolss .../wine/dlls/spoolss/tests/spoolss.c:218:5
    #3 0x000140003ede in run_test .../wine/include/wine/test.h:765:5
    #4 0x000140003a8e in main .../wine/include/wine/test.h
    #5 0x000140004963 in mainCRTStartup .../wine/dlls/msvcrt/crt_main.c:58:11
    #6 0x6fffffc3565e in BaseThreadInitThunk .../wine/dlls/kernel32/thread.c:61:24
    #7 0x6fffffdbbcc2 in RtlUserThreadStart (C:\windows\system32\ntdll.dll+0x17004bcc2)

Address 0x7ffffe1ffba0 is located in stack of thread T0 at offset 64 in frame
    #0 0x00014000164f in test_BuildOtherNamesFromMachineName .../wine/dlls/spoolss/tests/spoolss.c:122

  This frame has 2 object(s):
    [32, 40) 'buffers' (line 123)
    [64, 68) 'numentries' (line 124) <== Memory access at offset 64 partially 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 .../wine/dlls/spoolss/spoolss_main.c:99:11 in BuildOtherNamesFromMachineName
Shadow bytes around the buggy address:
  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 00 00 00 00 00 00 00 00
  0x7ffffe1ffa00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7ffffe1ffa80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x7ffffe1ffb00: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
=>0x7ffffe1ffb80: 00 f2 f2 f2[04]f3 f3 f3 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
  0x7ffffe1ffd80: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00
  0x7ffffe1ffe00: 00 00 00 00 00 00 00 00 00 00 00 00 f2 f2 f2 f2
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
==spoolss_test.exe==700==ABORTING
```

</details>

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

[PATCH 1/1] spoolss: Avoid buffer-overflow when setting numentries. (ASan)

[Bernhard Übelacker]

From: Bernhard Übelacker <bernhardu(a)mailbox.org>

---
 dlls/spoolss/spoolss_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dlls/spoolss/spoolss_main.c b/dlls/spoolss/spoolss_main.c
index 355bf72b822..aab280304f0 100644
--- a/dlls/spoolss/spoolss_main.c
+++ b/dlls/spoolss/spoolss_main.c
@@ -91,12 +91,12 @@ LPWSTR WINAPI AllocSplStr(LPCWSTR pwstr)
 /******************************************************************
  *   BuildOtherNamesFromMachineName   [SPOOLSS.@]
  */
-BOOL WINAPI BuildOtherNamesFromMachineName(LPVOID * ptr1, LPVOID * ptr2)
+BOOL WINAPI BuildOtherNamesFromMachineName(LPVOID * ptr1, LPDWORD ptr2)
 {
     FIXME("(%p, %p) stub\n", ptr1, ptr2);
 
     *ptr1 = NULL;
-    *ptr2 = NULL;
+    *ptr2 = 0;
     return FALSE;
 }
 
-- 
GitLab

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

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/spoolss/spoolss_main.c:
> /******************************************************************
>   *   BuildOtherNamesFromMachineName   [SPOOLSS.@]
>   */
> -BOOL WINAPI BuildOtherNamesFromMachineName(LPVOID * ptr1, LPVOID * ptr2)
> +BOOL WINAPI BuildOtherNamesFromMachineName(LPVOID * ptr1, LPDWORD ptr2)
BOOL WINAPI BuildOtherNamesFromMachineName(WCHAR ***ptr1, DWORD *ptr2)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7798#note_101444

[Huw Davies (＠huw)]

More generally, the spoolss tests aren't being run on anything >= Win7 and so aren't being run at all.  We should probably just remove them.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7798#note_101445