[PATCH 03/17] shell32/autocomplete: Handle heap_alloc failure and avoid a potential buffer overflow

Gabriel Ivăncescu gabrielopcode at gmail.com
Wed Sep 5 11:13:05 CDT 2018


Besides what's stated in the comments, the quickComplete format can have
stuff like %1234s which can make the resulting string much larger, and
that's handled via the while loop.

Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
---
 dlls/shell32/autocomplete.c | 59 +++++++++++++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 21 deletions(-)

diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c
index 254884b..ec91474 100644
--- a/dlls/shell32/autocomplete.c
+++ b/dlls/shell32/autocomplete.c
@@ -109,7 +109,6 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam,
     IAutoCompleteImpl *This = GetPropW(hwnd, autocomplete_propertyW);
     HRESULT hr;
     WCHAR hwndText[255];
-    WCHAR *hwndQCText;
     RECT r;
     BOOL control, filled, displayall = FALSE;
     int cpt, height, sel;
@@ -138,11 +137,27 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam,
                     /* If quickComplete is set and control is pressed, replace the string */
                     control = GetKeyState(VK_CONTROL) & 0x8000;
                     if (control && This->quickComplete) {
-                        hwndQCText = heap_alloc((lstrlenW(This->quickComplete)+lstrlenW(hwndText))*sizeof(WCHAR));
-                        sel = sprintfW(hwndQCText, This->quickComplete, hwndText);
-                        SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)hwndQCText);
-                        SendMessageW(hwnd, EM_SETSEL, 0, sel);
-                        heap_free(hwndQCText);
+                        WCHAR *qc;
+                        size_t sz = strlenW(This->quickComplete) + 1;
+                        sz += max(strlenW(hwndText), 2);  /* %s is 2 chars */
+
+                        while ((qc = heap_alloc(sz * sizeof(WCHAR)))) {
+                            /* Pass it 3 times to guard against crap like %*.*s, where the
+                               width and precision are taken from extra args preceeding it,
+                               so at least it won't crash or cause other vulnerabilities */
+                            sel = snprintfW(qc, sz, This->quickComplete,
+                                            hwndText, hwndText, hwndText);
+                            if (sel >= 0 && sel < sz) {
+                                SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)qc);
+                                SendMessageW(hwnd, EM_SETSEL, 0, sel);
+                                heap_free(qc);
+                                break;
+                            }
+                            heap_free(qc);
+                            if (sz > 0x800000U)  /* too large, give up */
+                                break;
+                            sz *= 2;
+                        }
                     }
 
                     ShowWindow(This->hwndListBox, SW_HIDE);
@@ -177,14 +192,14 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam,
                             SendMessageW(This->hwndListBox, LB_SETCURSEL, sel, 0);
                             if (sel != -1) {
                                 WCHAR *msg;
-                                int len;
-
-                                len = SendMessageW(This->hwndListBox, LB_GETTEXTLEN, sel, 0);
-                                msg = heap_alloc((len + 1)*sizeof(WCHAR));
-                                SendMessageW(This->hwndListBox, LB_GETTEXT, sel, (LPARAM)msg);
-                                SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)msg);
-                                SendMessageW(hwnd, EM_SETSEL, lstrlenW(msg), lstrlenW(msg));
-                                heap_free(msg);
+                                UINT len = SendMessageW(This->hwndListBox, LB_GETTEXTLEN, sel, 0);
+
+                                if ((msg = heap_alloc((len + 1)*sizeof(WCHAR)))) {
+                                    SendMessageW(This->hwndListBox, LB_GETTEXT, sel, (LPARAM)msg);
+                                    SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)msg);
+                                    SendMessageW(hwnd, EM_SETSEL, lstrlenW(msg), lstrlenW(msg));
+                                    heap_free(msg);
+                                }
                             } else {
                                 SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)This->txtbackup);
                                 SendMessageW(hwnd, EM_SETSEL, lstrlenW(This->txtbackup), lstrlenW(This->txtbackup));
@@ -291,7 +306,8 @@ static LRESULT APIENTRY ACLBoxSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam,
 {
     IAutoCompleteImpl *This = (IAutoCompleteImpl *)GetWindowLongPtrW(hwnd, GWLP_USERDATA);
     WCHAR *msg;
-    int sel, len;
+    UINT len;
+    int sel;
 
     switch (uMsg) {
         case WM_MOUSEMOVE:
@@ -303,12 +319,13 @@ static LRESULT APIENTRY ACLBoxSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam,
             if (sel < 0)
                 break;
             len = SendMessageW(This->hwndListBox, LB_GETTEXTLEN, sel, 0);
-            msg = heap_alloc((len + 1)*sizeof(WCHAR));
-            SendMessageW(hwnd, LB_GETTEXT, sel, (LPARAM)msg);
-            SendMessageW(This->hwndEdit, WM_SETTEXT, 0, (LPARAM)msg);
-            SendMessageW(This->hwndEdit, EM_SETSEL, 0, lstrlenW(msg));
-            ShowWindow(hwnd, SW_HIDE);
-            heap_free(msg);
+            if ((msg = heap_alloc((len + 1)*sizeof(WCHAR)))) {
+                SendMessageW(hwnd, LB_GETTEXT, sel, (LPARAM)msg);
+                SendMessageW(This->hwndEdit, WM_SETTEXT, 0, (LPARAM)msg);
+                SendMessageW(This->hwndEdit, EM_SETSEL, 0, lstrlenW(msg));
+                ShowWindow(hwnd, SW_HIDE);
+                heap_free(msg);
+            }
             break;
         default:
             return CallWindowProcW(This->wpOrigLBoxProc, hwnd, uMsg, wParam, lParam);
-- 
1.9.1




More information about the wine-devel mailing list