[PATCH v2] gdi32: Fail in ExtTextOut if count is larger than INT_MAX.
Some applications pass values like -1 and crash when BIDI_Reorder can't allocate the memory. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/gdi32/font.c | 3 +++ dlls/gdi32/tests/font.c | 2 ++ dlls/gdi32/tests/metafile.c | 8 +++++++- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/dlls/gdi32/font.c b/dlls/gdi32/font.c index 74ca482..de50bf0 100644 --- a/dlls/gdi32/font.c +++ b/dlls/gdi32/font.c @@ -5823,6 +5823,8 @@ BOOL WINAPI ExtTextOutA( HDC hdc, INT x, INT y, UINT flags, BOOL ret; LPINT lpDxW = NULL; + if (count > INT_MAX) return FALSE; + if (flags & ETO_GLYPH_INDEX) return ExtTextOutW( hdc, x, y, flags, lprect, (LPCWSTR)str, count, lpDx ); @@ -5932,6 +5934,7 @@ BOOL WINAPI ExtTextOutW( HDC hdc, INT x, INT y, UINT flags, static int quietfixme = 0; if (!dc) return FALSE; + if (count > INT_MAX) return FALSE; align = dc->textAlign; breakRem = dc->breakRem; diff --git a/dlls/gdi32/tests/font.c b/dlls/gdi32/tests/font.c index 461eecf..3e1f1e9 100644 --- a/dlls/gdi32/tests/font.c +++ b/dlls/gdi32/tests/font.c @@ -6916,6 +6916,8 @@ static void test_bitmap_font_glyph_index(void) hBmpPrev = SelectObject(hdc, hBmp[j]); switch (j) { case 0: + ret = ExtTextOutW(hdc, 0, 0, 0, NULL, text, -1, NULL); + ok(!ret, "ExtTextOutW succeeded\n"); ret = ExtTextOutW(hdc, 0, 0, 0, NULL, text, lstrlenW(text), NULL); break; case 1: diff --git a/dlls/gdi32/tests/metafile.c b/dlls/gdi32/tests/metafile.c index 8dae908..15af24a 100644 --- a/dlls/gdi32/tests/metafile.c +++ b/dlls/gdi32/tests/metafile.c @@ -222,7 +222,13 @@ static void test_ExtTextOut(void) ret = ExtTextOutA(hdcMetafile, 0, 40, 0, NULL, text, lstrlenA(text), NULL); ok( ret, "ExtTextOutA error %d\n", GetLastError()); - /* 4. test with unmatched BeginPath/EndPath calls */ + /* 4. pass -1 to length */ + SetLastError(0xdeadbeef); + ret = ExtTextOutA(hdcMetafile, 0, 0, 0, &rc, text, -1, NULL); + ok( !ret, "ExtTextOutA succeeded\n"); + ok( GetLastError() == 0xdeadbeef, "ExtTextOutA error %d\n", GetLastError()); + + /* 5. test with unmatched BeginPath/EndPath calls */ ret = BeginPath(hdcMetafile); ok( ret, "BeginPath error %d\n", GetLastError()); ret = BeginPath(hdcMetafile); -- 2.29.2
On Sat, Jan 30, 2021 at 04:02:09PM +0200, Gabriel Ivăncescu wrote:
Some applications pass values like -1 and crash when BIDI_Reorder can't allocate the memory.
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/gdi32/font.c | 3 +++ dlls/gdi32/tests/font.c | 2 ++ dlls/gdi32/tests/metafile.c | 8 +++++++- 3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/dlls/gdi32/font.c b/dlls/gdi32/font.c index 74ca482..de50bf0 100644 --- a/dlls/gdi32/font.c +++ b/dlls/gdi32/font.c @@ -5823,6 +5823,8 @@ BOOL WINAPI ExtTextOutA( HDC hdc, INT x, INT y, UINT flags, BOOL ret; LPINT lpDxW = NULL;
+ if (count > INT_MAX) return FALSE; +
What happens if ETO_OPAQUE and a valid rect are passed in this case? Does the rect get drawn? You could test this by adding such a call to draw_text_2() in gdi32/tests/dib.c
diff --git a/dlls/gdi32/tests/metafile.c b/dlls/gdi32/tests/metafile.c index 8dae908..15af24a 100644 --- a/dlls/gdi32/tests/metafile.c +++ b/dlls/gdi32/tests/metafile.c @@ -222,7 +222,13 @@ static void test_ExtTextOut(void) ret = ExtTextOutA(hdcMetafile, 0, 40, 0, NULL, text, lstrlenA(text), NULL); ok( ret, "ExtTextOutA error %d\n", GetLastError());
- /* 4. test with unmatched BeginPath/EndPath calls */ + /* 4. pass -1 to length */ + SetLastError(0xdeadbeef); + ret = ExtTextOutA(hdcMetafile, 0, 0, 0, &rc, text, -1, NULL); + ok( !ret, "ExtTextOutA succeeded\n"); + ok( GetLastError() == 0xdeadbeef, "ExtTextOutA error %d\n", GetLastError()); + + /* 5. test with unmatched BeginPath/EndPath calls */ ret = BeginPath(hdcMetafile); ok( ret, "BeginPath error %d\n", GetLastError()); ret = BeginPath(hdcMetafile);
It would be interesting to know whether the metafile record actually gets created in this case. Probably a stand-alone test at the end of this function would be easier. Likewise for EMFs. Huw.
Hi Huw, On 02/02/2021 10:54, Huw Davies wrote:
On Sat, Jan 30, 2021 at 04:02:09PM +0200, Gabriel Ivăncescu wrote:
Some applications pass values like -1 and crash when BIDI_Reorder can't allocate the memory.
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/gdi32/font.c | 3 +++ dlls/gdi32/tests/font.c | 2 ++ dlls/gdi32/tests/metafile.c | 8 +++++++- 3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/dlls/gdi32/font.c b/dlls/gdi32/font.c index 74ca482..de50bf0 100644 --- a/dlls/gdi32/font.c +++ b/dlls/gdi32/font.c @@ -5823,6 +5823,8 @@ BOOL WINAPI ExtTextOutA( HDC hdc, INT x, INT y, UINT flags, BOOL ret; LPINT lpDxW = NULL;
+ if (count > INT_MAX) return FALSE; +
What happens if ETO_OPAQUE and a valid rect are passed in this case? Does the rect get drawn? You could test this by adding such a call to draw_text_2() in gdi32/tests/dib.c
diff --git a/dlls/gdi32/tests/metafile.c b/dlls/gdi32/tests/metafile.c index 8dae908..15af24a 100644 --- a/dlls/gdi32/tests/metafile.c +++ b/dlls/gdi32/tests/metafile.c @@ -222,7 +222,13 @@ static void test_ExtTextOut(void) ret = ExtTextOutA(hdcMetafile, 0, 40, 0, NULL, text, lstrlenA(text), NULL); ok( ret, "ExtTextOutA error %d\n", GetLastError());
- /* 4. test with unmatched BeginPath/EndPath calls */ + /* 4. pass -1 to length */ + SetLastError(0xdeadbeef); + ret = ExtTextOutA(hdcMetafile, 0, 0, 0, &rc, text, -1, NULL); + ok( !ret, "ExtTextOutA succeeded\n"); + ok( GetLastError() == 0xdeadbeef, "ExtTextOutA error %d\n", GetLastError()); + + /* 5. test with unmatched BeginPath/EndPath calls */ ret = BeginPath(hdcMetafile); ok( ret, "BeginPath error %d\n", GetLastError()); ret = BeginPath(hdcMetafile);
It would be interesting to know whether the metafile record actually gets created in this case. Probably a stand-alone test at the end of this function would be easier. Likewise for EMFs.
Huw.
Good points. The patch seems to be correct, but I've sent a v3 with added tests for all these situations, except for normal metafiles. When I tested on Windows 7, ExtTextOutA crashed with -1 count if it was a normal Windows-format metafile, so I just added a comment for it.
participants (2)
-
Gabriel Ivăncescu -
Huw Davies