Commit caca4860 authored by Dylan Smith's avatar Dylan Smith Committed by Alexandre Julliard

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.
parent 4544efc9
...@@ -1906,20 +1906,16 @@ static int ME_GetTextEx(ME_TextEditor *editor, GETTEXTEX *ex, LPARAM pText) ...@@ -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) if (unicode) {
return ME_GetTextW(editor, rng->lpstrText, rng->chrg.cpMin, return ME_GetTextW(editor, strText, start, nLen, 0);
rng->chrg.cpMax-rng->chrg.cpMin, 0); } else {
else
{
int nLen = rng->chrg.cpMax-rng->chrg.cpMin;
WCHAR *p = ALLOC_N_OBJ(WCHAR, nLen+1); WCHAR *p = ALLOC_N_OBJ(WCHAR, nLen+1);
int nChars = ME_GetTextW(editor, p, rng->chrg.cpMin, nLen, 0); int nChars = ME_GetTextW(editor, p, start, nLen, 0);
/* FIXME this is a potential security hole (buffer overrun) WideCharToMultiByte(CP_ACP, 0, p, nChars+1, (char *)strText,
if you know more about wchar->mbyte conversion please explain nLen+1, NULL, NULL);
*/
WideCharToMultiByte(CP_ACP, 0, p, nChars+1, (char *)rng->lpstrText, nLen+1, NULL, NULL);
FREE_OBJ(p); FREE_OBJ(p);
return nChars; return nChars;
} }
...@@ -3580,12 +3576,9 @@ LRESULT ME_HandleMessage(ME_TextEditor *editor, UINT msg, WPARAM wParam, ...@@ -3580,12 +3576,9 @@ LRESULT ME_HandleMessage(ME_TextEditor *editor, UINT msg, WPARAM wParam,
case EM_GETSELTEXT: case EM_GETSELTEXT:
{ {
int from, to; int from, to;
TEXTRANGEW tr; /* W and A differ only by rng->lpstrText */
ME_GetSelection(editor, &from, &to); ME_GetSelection(editor, &from, &to);
tr.chrg.cpMin = from; return ME_GetTextRange(editor, (WCHAR *)lParam, from,
tr.chrg.cpMax = to; to - from, unicode);
tr.lpstrText = (WCHAR *)lParam;
return ME_GetTextRange(editor, &tr, unicode);
} }
case EM_GETSCROLLPOS: case EM_GETSCROLLPOS:
{ {
...@@ -3597,10 +3590,17 @@ LRESULT ME_HandleMessage(ME_TextEditor *editor, UINT msg, WPARAM wParam, ...@@ -3597,10 +3590,17 @@ LRESULT ME_HandleMessage(ME_TextEditor *editor, UINT msg, WPARAM wParam,
case EM_GETTEXTRANGE: case EM_GETTEXTRANGE:
{ {
TEXTRANGEW *rng = (TEXTRANGEW *)lParam; 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", TRACE("EM_GETTEXTRANGE min=%d max=%d unicode=%d emul1.0=%d length=%d\n",
rng->chrg.cpMin, rng->chrg.cpMax, unicode, rng->chrg.cpMin, rng->chrg.cpMax, unicode,
editor->bEmulateVersion10, ME_GetTextLength(editor)); 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: case EM_GETLINE:
{ {
......
...@@ -1420,6 +1420,46 @@ static void test_EM_GETTEXTRANGE(void) ...@@ -1420,6 +1420,46 @@ static void test_EM_GETTEXTRANGE(void)
ok(result == 7, "EM_GETTEXTRANGE returned %ld\n", result); ok(result == 7, "EM_GETTEXTRANGE returned %ld\n", result);
ok(!strcmp(expect, buffer), "EM_GETTEXTRANGE filled %s\n", buffer); 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); DestroyWindow(hwndRichEdit);
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment