Dylan Smith : richedit: Add bounds checks for EM_GETTEXTRANGE with tests.
Module: wine Branch: master Commit: caca4860290013b8fcf63a101d314346f870cbf4 URL: http://source.winehq.org/git/wine.git/?a=commit;h=caca4860290013b8fcf63a101d... Author: Dylan Smith <dylan.ah.smith(a)gmail.com> Date: Mon Mar 23 00:14:33 2009 -0400 richedit: Add bounds checks for EM_GETTEXTRANGE with tests. Wine was not doing bounds checks for EM_GETTEXTRANGE, which was causing a crash in Bug 17822. The added tests would cause a crash without the added bounds checks in the richedit code. The bounds checks I put in HandleMessage, since ME_GetTextRange is also called for ME_GETSELTEXT which should not have bounds checks, since it uses the selection range. When the ME_GETTEXTRANGE message returns 0, no text is copied, not even the NULL terminating charter. This differs from EM_GETSELTEXT which will copy the NULL terminating character when no text is selected. This behaviour is consistent with native richedit controls. --- dlls/riched20/editor.c | 36 ++++++++++++++++++------------------ dlls/riched20/tests/editor.c | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 18 deletions(-) diff --git a/dlls/riched20/editor.c b/dlls/riched20/editor.c index c9cb244..00fc38e 100644 --- a/dlls/riched20/editor.c +++ b/dlls/riched20/editor.c @@ -1906,20 +1906,16 @@ static int ME_GetTextEx(ME_TextEditor *editor, GETTEXTEX *ex, LPARAM pText) } } -static int ME_GetTextRange(ME_TextEditor *editor, TEXTRANGEW *rng, BOOL unicode) +static int ME_GetTextRange(ME_TextEditor *editor, WCHAR *strText, + int start, int nLen, BOOL unicode) { - if (unicode) - return ME_GetTextW(editor, rng->lpstrText, rng->chrg.cpMin, - rng->chrg.cpMax-rng->chrg.cpMin, 0); - else - { - int nLen = rng->chrg.cpMax-rng->chrg.cpMin; + if (unicode) { + return ME_GetTextW(editor, strText, start, nLen, 0); + } else { WCHAR *p = ALLOC_N_OBJ(WCHAR, nLen+1); - int nChars = ME_GetTextW(editor, p, rng->chrg.cpMin, nLen, 0); - /* FIXME this is a potential security hole (buffer overrun) - if you know more about wchar->mbyte conversion please explain - */ - WideCharToMultiByte(CP_ACP, 0, p, nChars+1, (char *)rng->lpstrText, nLen+1, NULL, NULL); + int nChars = ME_GetTextW(editor, p, start, nLen, 0); + WideCharToMultiByte(CP_ACP, 0, p, nChars+1, (char *)strText, + nLen+1, NULL, NULL); FREE_OBJ(p); return nChars; } @@ -3580,12 +3576,9 @@ LRESULT ME_HandleMessage(ME_TextEditor *editor, UINT msg, WPARAM wParam, case EM_GETSELTEXT: { int from, to; - TEXTRANGEW tr; /* W and A differ only by rng->lpstrText */ ME_GetSelection(editor, &from, &to); - tr.chrg.cpMin = from; - tr.chrg.cpMax = to; - tr.lpstrText = (WCHAR *)lParam; - return ME_GetTextRange(editor, &tr, unicode); + return ME_GetTextRange(editor, (WCHAR *)lParam, from, + to - from, unicode); } case EM_GETSCROLLPOS: { @@ -3597,10 +3590,17 @@ LRESULT ME_HandleMessage(ME_TextEditor *editor, UINT msg, WPARAM wParam, case EM_GETTEXTRANGE: { TEXTRANGEW *rng = (TEXTRANGEW *)lParam; + int start = rng->chrg.cpMin; + int end = rng->chrg.cpMax; + int textlength = ME_GetTextLength(editor); TRACE("EM_GETTEXTRANGE min=%d max=%d unicode=%d emul1.0=%d length=%d\n", rng->chrg.cpMin, rng->chrg.cpMax, unicode, editor->bEmulateVersion10, ME_GetTextLength(editor)); - return ME_GetTextRange(editor, rng, unicode); + if (start < 0) return 0; + if ((start == 0 && end == -1) || end > textlength) + end = textlength; + if (start >= end) return 0; + return ME_GetTextRange(editor, rng->lpstrText, start, end - start, unicode); } case EM_GETLINE: { diff --git a/dlls/riched20/tests/editor.c b/dlls/riched20/tests/editor.c index 331fd69..40d0045 100644 --- a/dlls/riched20/tests/editor.c +++ b/dlls/riched20/tests/editor.c @@ -1420,6 +1420,46 @@ static void test_EM_GETTEXTRANGE(void) ok(result == 7, "EM_GETTEXTRANGE returned %ld\n", result); ok(!strcmp(expect, buffer), "EM_GETTEXTRANGE filled %s\n", buffer); + /* cpMax of text length is used instead of -1 in this case */ + textRange.lpstrText = buffer; + textRange.chrg.cpMin = 0; + textRange.chrg.cpMax = -1; + result = SendMessage(hwndRichEdit, EM_GETTEXTRANGE, 0, (LPARAM)&textRange); + ok(result == strlen(text2), "EM_GETTEXTRANGE returned %ld\n", result); + ok(!strcmp(text2, buffer), "EM_GETTEXTRANGE filled %s\n", buffer); + + /* cpMin < 0 causes no text to be copied, and 0 to be returned */ + textRange.lpstrText = buffer; + textRange.chrg.cpMin = -1; + textRange.chrg.cpMax = 1; + result = SendMessage(hwndRichEdit, EM_GETTEXTRANGE, 0, (LPARAM)&textRange); + ok(result == 0, "EM_GETTEXTRANGE returned %ld\n", result); + ok(!strcmp(text2, buffer), "EM_GETTEXTRANGE filled %s\n", buffer); + + /* cpMax of -1 is not replaced with text length if cpMin != 0 */ + textRange.lpstrText = buffer; + textRange.chrg.cpMin = 1; + textRange.chrg.cpMax = -1; + result = SendMessage(hwndRichEdit, EM_GETTEXTRANGE, 0, (LPARAM)&textRange); + ok(result == 0, "EM_GETTEXTRANGE returned %ld\n", result); + ok(!strcmp(text2, buffer), "EM_GETTEXTRANGE filled %s\n", buffer); + + /* no end character is copied if cpMax - cpMin < 0 */ + textRange.lpstrText = buffer; + textRange.chrg.cpMin = 5; + textRange.chrg.cpMax = 5; + result = SendMessage(hwndRichEdit, EM_GETTEXTRANGE, 0, (LPARAM)&textRange); + ok(result == 0, "EM_GETTEXTRANGE returned %ld\n", result); + ok(!strcmp(text2, buffer), "EM_GETTEXTRANGE filled %s\n", buffer); + + /* cpMax of text length is used if cpMax > text length*/ + textRange.lpstrText = buffer; + textRange.chrg.cpMin = 0; + textRange.chrg.cpMax = 1000; + result = SendMessage(hwndRichEdit, EM_GETTEXTRANGE, 0, (LPARAM)&textRange); + ok(result == strlen(text2), "EM_GETTEXTRANGE returned %ld\n", result); + ok(!strcmp(text2, buffer), "EM_GETTEXTRANGE filled %s\n", buffer); + DestroyWindow(hwndRichEdit); }
participants (1)
-
Alexandre Julliard