[PATCH 0/1] MR1207: comctl32/edit: Prevent a use after free

[Mark Jansen (＠learn-more)]

This was observed in ReactOS, when 'Debug Page Heap'[^1] was enabled:
- Start renaming a file in a shell view
- Cancel the renaming


[^1]: DPH is modeled after 'Windows Page Heap Verification'

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

[PATCH 1/1] comctl32/edit: Prevent a use after free

[Mark Jansen]

From: Mark Jansen <mark.jansen(a)reactos.org>

---
 dlls/comctl32/edit.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/dlls/comctl32/edit.c b/dlls/comctl32/edit.c
index 3111cda2a4b..d9831efa2ad 100644
--- a/dlls/comctl32/edit.c
+++ b/dlls/comctl32/edit.c
@@ -3403,12 +3403,18 @@ static LRESULT EDIT_WM_KeyDown(EDITSTATE *es, INT key)
 static LRESULT EDIT_WM_KillFocus(HTHEME theme, EDITSTATE *es)
 {
     UINT flags = RDW_INVALIDATE;
+    HWND hwndSelf = es->hwndSelf;
 
     es->flags &= ~EF_FOCUSED;
     DestroyCaret();
     if (!(es->style & ES_NOHIDESEL))
         EDIT_InvalidateText(es, es->selection_start, es->selection_end);
     EDIT_NOTIFY_PARENT(es, EN_KILLFOCUS);
+
+    /* Someone destroyed us in the EN_KILLFOCUS handler, so we can't use 'es' anymore */
+    if (!IsWindow(hwndSelf))
+        return 0;
+
     /* Throw away left over scroll when we lose focus */
     es->wheelDeltaRemainder = 0;
 
-- 
GitLab

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

[Nikolay Sivov (＠nsivov)]

I'd rather we used something similar to Listview helper, instead of a macro. This helper checks for valid window on return, for every notifications.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1207#note_12672

[Nikolay Sivov (＠nsivov)]

After that the same fix has to be made for user32/edit.c.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1207#note_12673

[Nikolay Sivov (＠nsivov)]

Please check if this works for you.

[0001-comctl32-edit-Check-for-control-destruction-on-notifca.txt](/uploads/b0ec758262ac76ef3fe0677551679fcd/0001-comctl32-edit-Check-for-control-destruction-on-notifca.txt)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1207#note_12717

[Mark Jansen (＠learn-more)]

This patch does work for me, thanks!

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1207#note_13181

[Nikolay Sivov (＠nsivov)]

I opened !1270 that applies this change to both implementations.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1207#note_13338

[PATCH 0/1] MR1207: comctl32/edit: Prevent a use after free - closed

[Nikolay Sivov (＠nsivov)]

This merge request was closed by Nikolay Sivov.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1207

[Nikolay Sivov (＠nsivov)]

Closing.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1207#note_13572