richedit: Add bounds checks for EM_GETTEXTRANGE with tests. (Bug 17822)

[Dylan Smith]

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(-)
-------------- next part --------------
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);
 }