Francois Gouget : user32: Fix a SetClipboardData() buffer overflow.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: 605ecafa67a4e034328b69396581e2f9b60d7af3
URL:    https://gitlab.winehq.org/wine/wine/-/commit/605ecafa67a4e034328b69396581e2f...

Author: Francois Gouget 
Date:   Wed Dec 21 18:48:29 2022 +0100

user32: Fix a SetClipboardData() buffer overflow.

Wine would append a correctly aligned NUL Unicode character to
terminate the string but overflow the buffer by one byte for odd-sized
strings.
Windows instead overwrites the last two buffer bytes with a NUL Unicode
character which ends up being misaligned for odd-sized strings.
The clipboard data has a size field anyway so match the Windows
behavior.
Tweak the tests to show that SetClipboardData() can overwrite half of
the Unicode string's last character.

---

dlls/user32/clipboard.c       |  5 +++--
 dlls/user32/tests/clipboard.c | 14 +++++++-------
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/dlls/user32/clipboard.c b/dlls/user32/clipboard.c
index f006e205e05..72c7720dc17 100644
--- a/dlls/user32/clipboard.c
+++ b/dlls/user32/clipboard.c
@@ -157,12 +157,13 @@ static HANDLE marshal_data( UINT format, HANDLE handle, size_t *ret_size )
         }
     case CF_UNICODETEXT:
         {
-            WCHAR *ptr;
+            char *ptr;
             if (!(size = GlobalSize( handle ))) return 0;
             if ((data_size_t)size != size) return 0;
             if (size < sizeof(WCHAR)) return 0;
             if (!(ptr = GlobalLock( handle ))) return 0;
-            ptr[(size + 1) / sizeof(WCHAR) - 1] = 0;  /* enforce null-termination */
+            /* enforce nul-termination the Windows way: ignoring alignment */
+            *((WCHAR *)(ptr + size) - 1) = 0;
             GlobalUnlock( handle );
             *ret_size = size;
             return handle;
diff --git a/dlls/user32/tests/clipboard.c b/dlls/user32/tests/clipboard.c
index 38249c24ea0..87df06b6621 100644
--- a/dlls/user32/tests/clipboard.c
+++ b/dlls/user32/tests/clipboard.c
@@ -2224,11 +2224,11 @@ static const struct
     { "", {}, 0 },
     { "", {'f'}, 1 },                 /* 5 */
     { "", {'f'}, 2 },
-    { "", {'f','o','o'}, 5 },
-    { "", {'f','o','o'}, 6 },
-    { "", {'f','o','o',0}, 7 },       /* 10 */
-    { "", {'f','o','o',0}, 8 },
-    { "", {'f','o','o',0,'b'}, 9 },
+    { "", {0x3b1,0x3b2,0x3b3}, 5 }, /* Alpha, beta, ... */
+    { "", {0x3b1,0x3b2,0x3b3}, 6 },
+    { "", {0x3b1,0x3b2,0x3b3,0}, 7 }, /* 10 */
+    { "", {0x3b1,0x3b2,0x3b3,0}, 8 },
+    { "", {0x3b1,0x3b2,0x3b3,0,0x3b4}, 9 },
     { "", {'f','o','o',0,'b','a','r'}, 7 * sizeof(WCHAR) },
     { "", {'f','o','o',0,'b','a','r',0}, 8 * sizeof(WCHAR) },
 };
@@ -2274,7 +2274,7 @@ static void test_string_data(void)
             {
                 ok( clip == data, "SetClipboardData() returned %p != %p\n", clip, data );
                 memcpy( bufferW, test_data[i].strW, test_data[i].len );
-                bufferW[(test_data[i].len + 1) / sizeof(WCHAR) - 1] = 0;
+                *((WCHAR *)((char *)bufferW + test_data[i].len) - 1) = 0;
                 ok( !memcmp( data, bufferW, test_data[i].len ),
                     "wrong data %s\n", wine_dbgstr_an( data, test_data[i].len ));
             }
@@ -2336,7 +2336,7 @@ static void test_string_data_process( int i )
             len = GlobalSize( data );
             ok( len == test_data[i].len, "wrong size %u / %u\n", len, test_data[i].len );
             memcpy( bufferW, test_data[i].strW, test_data[i].len );
-            bufferW[(test_data[i].len + 1) / sizeof(WCHAR) - 1] = 0;
+            *((WCHAR *)((char *)bufferW + test_data[i].len) - 1) = 0;
             ok( !memcmp( data, bufferW, len ), "wrong data %s\n", wine_dbgstr_an( data, len ));
         }