[PATCH 07/17] shell32/autocomplete: Redesign the window proc to trigger on key presses, handle arbitrary sizes and avoid another buffer overflow
Gabriel Ivăncescu
gabrielopcode at gmail.com
Wed Sep 5 11:13:09 CDT 2018
AutoComplete currently shows up when the user releases a key, which is
wrong. Windows does it when the user presses a key, so use both WM_KEYDOWN
and WM_CHAR and redesign it so that it matches Windows behavior.
At the same time, dynamically allocate the internal text buffer itself so we
can handle any length, while avoiding the buffer overflow. The previous code
caps the text at 255 characters (and in such case gives the wrong results),
and is prone to exploits, as it assumes the enumerated string fits in 255
characters as well, which can be easily exploited.
Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
---
This patch does many changes because they are either minor or intertwined
and hard to separate without breaking the compilation or Wine. A difficulty
here stems from the fact that we need to handle not just WM_KEYDOWN but
also WM_CHAR, which is harder than WM_KEYUP especially when trying to
share code paths because control characters have to be handled separately
(for auto-append purposes).
For example VK_BACK is not processed in WM_KEYDOWN because it's done during
WM_CHAR as the escape '\b' along the rest. That's not the case with VK_DELETE
which has no WM_CHAR escape. This is important since the edit control *must*
receive the message before us, otherwise the behavior will be wrong, so
they have to be deferred to WM_CHAR and we can't do it in WM_KEYDOWN. That's
also why we always make sure to pass the corresponding message to the edit
control first.
dlls/shell32/autocomplete.c | 169 ++++++++++++++++++++++++++------------------
1 file changed, 101 insertions(+), 68 deletions(-)
diff --git a/dlls/shell32/autocomplete.c b/dlls/shell32/autocomplete.c
index e5712e6..74728e0 100644
--- a/dlls/shell32/autocomplete.c
+++ b/dlls/shell32/autocomplete.c
@@ -108,17 +108,19 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam,
{
IAutoCompleteImpl *This = GetPropW(hwnd, autocomplete_propertyW);
HRESULT hr;
- WCHAR hwndText[255];
- RECT r;
- BOOL control, filled, displayall = FALSE;
- int cpt, height, sel;
+ LRESULT ret;
+ WCHAR *hwndText;
+ UINT len, old_len, cpt;
+ BOOLEAN displayall = FALSE, noautoappend = !(This->options & ACO_AUTOAPPEND);
+ INT sel;
if (!This->enabled) return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
switch (uMsg)
{
case CB_SHOWDROPDOWN:
- ShowWindow(This->hwndListBox, SW_HIDE);
+ if (This->options & ACO_AUTOSUGGEST)
+ ShowWindow(This->hwndListBox, SW_HIDE);
break;
case WM_KILLFOCUS:
if ((This->options & ACO_AUTOSUGGEST) && ((HWND)wParam != This->hwndListBox))
@@ -126,21 +128,19 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam,
ShowWindow(This->hwndListBox, SW_HIDE);
}
return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
- case WM_KEYUP:
- {
- int len;
-
- GetWindowTextW(hwnd, hwndText, ARRAY_SIZE(hwndText));
-
+ case WM_KEYDOWN:
switch(wParam) {
case VK_RETURN:
/* If quickComplete is set and control is pressed, replace the string */
- control = GetKeyState(VK_CONTROL) & 0x8000;
- if (control && This->quickComplete) {
+ if (This->quickComplete && (GetKeyState(VK_CONTROL) & 0x8000)) {
WCHAR *qc;
- size_t sz = strlenW(This->quickComplete) + 1;
- sz += max(strlenW(hwndText), 2); /* %s is 2 chars */
+ size_t sz;
+ len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0) + 1; /* include NUL */
+ if (!(hwndText = heap_alloc(len * sizeof(WCHAR))))
+ return 0;
+ len = SendMessageW(hwnd, WM_GETTEXT, len, (LPARAM)hwndText);
+ sz = strlenW(This->quickComplete)+1 + max(len, 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,
@@ -158,31 +158,35 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam,
break;
sz *= 2;
}
+ if (This->options & ACO_AUTOSUGGEST)
+ ShowWindow(This->hwndListBox, SW_HIDE);
+ heap_free(hwndText);
+ return 0;
}
- ShowWindow(This->hwndListBox, SW_HIDE);
- return 0;
- case VK_LEFT:
- case VK_RIGHT:
- return 0;
+ if (This->options & ACO_AUTOSUGGEST)
+ ShowWindow(This->hwndListBox, SW_HIDE);
+ break;
case VK_UP:
case VK_DOWN:
- /* Two cases here :
- - if the listbox is not visible, displays it
- with all the entries if the style ACO_UPDOWNKEYDROPSLIST
- is present but does not select anything.
+ /* Two cases here:
+ - if the listbox is not visible and ACO_UPDOWNKEYDROPSLIST is
+ set, display it with all the entries, without selecting any
- if the listbox is visible, change the selection
*/
- if ( (This->options & (ACO_AUTOSUGGEST | ACO_UPDOWNKEYDROPSLIST))
- && (!IsWindowVisible(This->hwndListBox) && (! *hwndText)) )
- {
- /* We must display all the entries */
- displayall = TRUE;
- } else {
- if (IsWindowVisible(This->hwndListBox)) {
- int count;
+ if (This->options & ACO_AUTOSUGGEST) {
+ if (!IsWindowVisible(This->hwndListBox)) {
+ if (This->options & ACO_UPDOWNKEYDROPSLIST) {
+ ret = 0;
+ displayall = TRUE;
+ noautoappend = TRUE;
+ goto autocomplete_text;
+ }
+ } else {
+ INT count;
count = SendMessageW(This->hwndListBox, LB_GETCOUNT, 0, 0);
+
/* Change the selection */
sel = SendMessageW(This->hwndListBox, LB_GETCURSEL, 0, 0);
if (wParam == VK_UP)
@@ -206,34 +210,51 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam,
len = strlenW(This->txtbackup);
SendMessageW(hwnd, EM_SETSEL, len, len);
}
+ return 0;
}
- return 0;
}
break;
- case VK_BACK:
case VK_DELETE:
- if ((! *hwndText) && (This->options & ACO_AUTOSUGGEST)) {
- ShowWindow(This->hwndListBox, SW_HIDE);
- return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
- }
- break;
- default:
- ;
+ ret = CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
+ goto handle_control_char;
+ }
+ return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
+ case WM_CHAR:
+ case WM_UNICHAR:
+ ret = CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
+ if (wParam < ' ')
+ {
+ handle_control_char:
+ len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0);
+ if ((This->options & ACO_AUTOSUGGEST) && len == 0) {
+ ShowWindow(This->hwndListBox, SW_HIDE);
+ return ret;
+ }
+ if (wParam != 0x16 /* ^V (paste) */)
+ noautoappend = TRUE;
}
+ else {
+ autocomplete_text:
+ len = SendMessageW(hwnd, WM_GETTEXTLENGTH, 0, 0);
+ }
+
+ old_len = len+1; /* include NUL */
+ if (!(hwndText = heap_alloc(old_len * sizeof(WCHAR))))
+ return ret;
+ len = SendMessageW(hwnd, WM_GETTEXT, old_len, (LPARAM)hwndText);
+ if (len+1 != old_len)
+ hwndText = heap_realloc(hwndText, len+1);
SendMessageW(This->hwndListBox, LB_RESETCONTENT, 0, 0);
+ /* Set txtbackup to point to hwndText itself (which must not be released) */
heap_free(This->txtbackup);
- len = strlenW(hwndText);
- This->txtbackup = heap_alloc((len + 1)*sizeof(WCHAR));
- lstrcpyW(This->txtbackup, hwndText);
+ This->txtbackup = hwndText;
- /* Returns if there is no text to search and we doesn't want to display all the entries */
- if ((!displayall) && (! *hwndText) )
- break;
+ if (!displayall && !len)
+ return ret;
IEnumString_Reset(This->enumstr);
- filled = FALSE;
for(cpt = 0;;) {
LPOLESTR strs = NULL;
ULONG fetched;
@@ -243,62 +264,68 @@ static LRESULT APIENTRY ACEditSubclassProc(HWND hwnd, UINT uMsg, WPARAM wParam,
break;
if (!strncmpiW(hwndText, strs, len)) {
- if (!filled && (This->options & ACO_AUTOAPPEND) &&
- wParam != VK_BACK && wParam != VK_DELETE)
+ if (cpt == 0 && noautoappend == FALSE)
{
- WCHAR buffW[255];
+ /* The character capitalization can be different,
+ so merge hwndText and strs into a new string */
+ WCHAR *tmp;
+ size_t strslen = len + strlenW(&strs[len]);
+
+ if ((tmp = heap_alloc((strslen+1) * sizeof(WCHAR))))
+ {
+ memcpy(tmp, hwndText, len * sizeof(WCHAR));
+ memcpy(&tmp[len], &strs[len], (strslen-len+1) * sizeof(WCHAR));
+ }
+ else tmp = strs;
+
+ SendMessageW(hwnd, WM_SETTEXT, 0, (LPARAM)tmp);
+ SendMessageW(hwnd, EM_SETSEL, len, strslen);
+ if (tmp != strs)
+ heap_free(tmp);
- strcpyW(buffW, hwndText);
- strcatW(buffW, &strs[len]);
- SetWindowTextW(hwnd, buffW);
- SendMessageW(hwnd, EM_SETSEL, len, strlenW(strs));
if (!(This->options & ACO_AUTOSUGGEST)) {
CoTaskMemFree(strs);
break;
}
}
- if (This->options & ACO_AUTOSUGGEST) {
- SendMessageW(This->hwndListBox, LB_ADDSTRING, 0, (LPARAM)strs);
- cpt++;
- }
+ if (This->options & ACO_AUTOSUGGEST)
+ SendMessageW(This->hwndListBox, LB_INSERTSTRING, -1, (LPARAM)strs);
- filled = TRUE;
+ cpt++;
}
CoTaskMemFree(strs);
}
if (This->options & ACO_AUTOSUGGEST) {
- if (filled) {
- height = SendMessageW(This->hwndListBox, LB_GETITEMHEIGHT, 0, 0);
+ if (cpt) {
+ RECT r;
+ UINT height = SendMessageW(This->hwndListBox, LB_GETITEMHEIGHT, 0, 0);
SendMessageW(This->hwndListBox, LB_CARETOFF, 0, 0);
GetWindowRect(hwnd, &r);
SetParent(This->hwndListBox, HWND_DESKTOP);
/* It seems that Windows XP displays 7 lines at most
and otherwise displays a vertical scroll bar */
SetWindowPos(This->hwndListBox, HWND_TOP,
- r.left, r.bottom + 1, r.right - r.left, min(height * 7, height*(cpt+1)),
+ r.left, r.bottom + 1, r.right - r.left, height*min(cpt+1, 7),
SWP_SHOWWINDOW );
} else {
ShowWindow(This->hwndListBox, SW_HIDE);
}
}
-
- break;
- }
+ return ret;
case WM_DESTROY:
{
WNDPROC proc = This->wpOrigEditProc;
- RemovePropW(hwnd, autocomplete_propertyW);
SetWindowLongPtrW(hwnd, GWLP_WNDPROC, (LONG_PTR)proc);
+ RemovePropW(hwnd, autocomplete_propertyW);
destroy_autocomplete_object(This);
return CallWindowProcW(proc, hwnd, uMsg, wParam, lParam);
}
default:
return CallWindowProcW(This->wpOrigEditProc, hwnd, uMsg, wParam, lParam);
-
}
return 0;
@@ -351,6 +378,8 @@ static void create_listbox(IAutoCompleteImpl *This)
This->wpOrigLBoxProc = (WNDPROC) SetWindowLongPtrW( This->hwndListBox, GWLP_WNDPROC, (LONG_PTR) ACLBoxSubclassProc);
SetWindowLongPtrW( This->hwndListBox, GWLP_USERDATA, (LONG_PTR)This);
}
+ else
+ This->options &= ~ACO_AUTOSUGGEST;
}
/**************************************************************************
@@ -470,6 +499,10 @@ static HRESULT WINAPI IAutoComplete2_fnInit(
return This->hwndEdit ? E_FAIL : E_UNEXPECTED;
}
+ /* Prevent txtbackup from ever being NULL to simplify the code */
+ if ((This->txtbackup = heap_alloc_zero(sizeof(WCHAR))) == NULL)
+ return E_OUTOFMEMORY;
+
if (FAILED (IUnknown_QueryInterface (punkACL, &IID_IEnumString, (LPVOID*)&This->enumstr))) {
WARN("No IEnumString interface\n");
return E_NOINTERFACE;
--
1.9.1
More information about the wine-devel
mailing list