[PATCH 0/1] MR1147: win32u: Make sure that the stack buffer in set_multi_value_key is large enough

[Martin Storsjö]

This fixes stack overflows since
edecac8afdd04701314381b6386e0f7ac862e066.

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

[PATCH 1/1] win32u: Make sure that the stack buffer in set_multi_value_key is large enough

[Martin Storsjö]

From: Martin Storsjö <martin(a)martin.st>

This fixes stack overflows since
edecac8afdd04701314381b6386e0f7ac862e066.

Signed-off-by: Martin Storsjö <martin(a)martin.st>
---
 dlls/win32u/font.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/dlls/win32u/font.c b/dlls/win32u/font.c
index 9ead47e1983..6919edb2915 100644
--- a/dlls/win32u/font.c
+++ b/dlls/win32u/font.c
@@ -2919,7 +2919,8 @@ static void update_font_association_info(void)
 
 static void set_multi_value_key( HKEY hkey, const WCHAR *name, const char *value, DWORD len )
 {
-    WCHAR valueW[256];
+    WCHAR valueW[512];
+    assert(len <= sizeof(valueW)/sizeof(WCHAR));
     ascii_to_unicode( valueW, value, len );
     if (value)
         set_reg_value( hkey, name, REG_MULTI_SZ, valueW, len * sizeof(WCHAR) );
-- 
GitLab

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

[PATCH 1/1] win32u: Make sure that the stack buffer in set_multi_value_key is large enough

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=125335

Your paranoid android.


=== debian11 (32 bit report) ===

ws2_32:
protocol.c:2437: Test succeeded inside todo block: IPv6 address is returned.
protocol.c:2454: Test succeeded inside todo block: Test 0: IPv6 address is returned.
protocol.c:2454: Test succeeded inside todo block: Test 1: IPv6 address is returned.
protocol.c:2454: Test succeeded inside todo block: Test 2: IPv6 address is returned.
protocol.c:2454: Test succeeded inside todo block: Test 3: IPv6 address is returned.
protocol.c:2454: Test succeeded inside todo block: Test 4: IPv6 address is returned.
protocol.c:2454: Test succeeded inside todo block: Test 5: IPv6 address is returned.
protocol.c:2454: Test succeeded inside todo block: Test 12: IPv6 address is returned.
protocol.c:2454: Test succeeded inside todo block: Test 13: IPv6 address is returned.
protocol.c:2454: Test succeeded inside todo block: Test 14: IPv6 address is returned.
protocol.c:2454: Test succeeded inside todo block: Test 15: IPv6 address is returned.
protocol.c:2454: Test succeeded inside todo block: Test 16: IPv6 address is returned.
protocol.c:2454: Test succeeded inside todo block: Test 28: IPv6 address is returned.
protocol.c:2454: Test succeeded inside todo block: Test 29: IPv6 address is returned.

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/win32u/font.c:
> static void set_multi_value_key( HKEY hkey, const WCHAR *name, const char *value, DWORD len )
>  {
> -    WCHAR valueW[256];
> +    WCHAR valueW[512];
> +    assert(len <= sizeof(valueW)/sizeof(WCHAR));

Since this isn't in a hot path, let's dynamically allocate `valueW`.

Also note for future reference `ARRAY_SIZE(array)` instead of `sizeof(array) / sizeof(array[0])`.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1147#note_11859