[PATCH v2 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.

--
  v2: win32u: Make sure that the stack buffer in set_multi_value_key is large enough
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1147

[PATCH v2 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 | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/dlls/win32u/font.c b/dlls/win32u/font.c
index 9ead47e1983..5c9e2acbaf2 100644
--- a/dlls/win32u/font.c
+++ b/dlls/win32u/font.c
@@ -2919,12 +2919,18 @@ 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;
+
+    if (!(valueW = malloc( len * sizeof(WCHAR) ))) {
+        ERR("malloc of %d * WCHAR failed\n", len);
+        return;
+    }
     ascii_to_unicode( valueW, value, len );
     if (value)
         set_reg_value( hkey, name, REG_MULTI_SZ, valueW, len * sizeof(WCHAR) );
     else if (name)
         reg_delete_value( hkey, name );
+    free(valueW);
 }
 
 static void update_font_system_link_info(void)
-- 
GitLab

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

[Martin Storsjö]

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

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

Ah, yes. Thanks!

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

[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;
> +
> +    if (!(valueW = malloc( len * sizeof(WCHAR) ))) {
> +        ERR("malloc of %d * WCHAR failed\n", len);

```suggestion:-1+0
    if (!(valueW = malloc( len * sizeof(WCHAR) )))
    {
        ERR( "malloc of %d * WCHAR failed\n", len );
```

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

[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;
> +
> +    if (!(valueW = malloc( len * sizeof(WCHAR) ))) {
> +        ERR("malloc of %d * WCHAR failed\n", len);
> +        return;
> +    }
>      ascii_to_unicode( valueW, value, len );
>      if (value)
>          set_reg_value( hkey, name, REG_MULTI_SZ, valueW, len * sizeof(WCHAR) );
>      else if (name)
>          reg_delete_value( hkey, name );
> +    free(valueW);

```suggestion:-0+0
    free( valueW );
```

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