[PATCH 0/1] MR6650: comdlg32: Allocate extbuf on heap rather than stack.

[Charlotte Pabst (＠CharlottePabst)]

This fixes a stack overflow in Helicon Focus 8.2.0,
which was introduced by b5cbb556.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57293

I'm unsure if this is actually the proper way to approach this problem, since an application being so close to a stack overflow that an allocation of wchar_t[MAX_PATH] (=260*2 bytes) is making the difference between crashing and not crashing might be an issue in itself.

However, the crash was introduced by adding this allocation (or rather, changing the condition for it to happen) in previous commit, so for now I think the proper solution would be to move it to the heap instead.

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

[PATCH 1/1] comdlg32: Allocate extbuf on heap rather than stack.

[Charlotte Pabst]

From: Charlotte Pabst <cpabst(a)codeweavers.com>

This fixes a stack overflow in Helicon Focus 8.2.0,
which was introduced by b5cbb556.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57293
---
 dlls/comdlg32/itemdlg.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/dlls/comdlg32/itemdlg.c b/dlls/comdlg32/itemdlg.c
index 784a14b47e4..b3f947c85b6 100644
--- a/dlls/comdlg32/itemdlg.c
+++ b/dlls/comdlg32/itemdlg.c
@@ -646,7 +646,7 @@ static HRESULT on_default_action(FileDialogImpl *This)
         /* Add the proper extension */
         if(open_action == ONOPEN_OPEN)
         {
-            WCHAR extbuf[MAX_PATH], *newext = NULL;
+            WCHAR *extbuf = LocalAlloc(0, MAX_PATH * sizeof *extbuf), *newext = NULL;
 
             if(This->current_filter)
             {
@@ -691,6 +691,8 @@ static HRESULT on_default_action(FileDialogImpl *This)
                     }
                 }
             }
+
+            LocalFree(extbuf);
         } else if (open_action == ONOPEN_SEARCH) {
             filter = fn_iter;
         }
-- 
GitLab

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

[Alfred Agrell (＠Alcaro)]

Needs a comment, or someone's gonna change it back as an optimization.

And needs to do something if the allocation fails.

Did you check what's on the stack when it crashes? Is this specific function going recursive somehow? Was the thread created with an 8KB stack? Is the app allocating some megabyte-sized chunks on the stack, and just barely squeezing by on native too?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6650#note_84673