[PATCH v2 0/1] MR6198: gdi32: Bounds check EMF handle tables.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45106 -- v2: gdi32: Bounds check EMF handle tables. https://gitlab.winehq.org/wine/wine/-/merge_requests/6198
From: Esme Povirk <esme(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45106 --- dlls/gdi32/enhmetafile.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/dlls/gdi32/enhmetafile.c b/dlls/gdi32/enhmetafile.c index 0ef26522eb6..89622ef1b58 100644 --- a/dlls/gdi32/enhmetafile.c +++ b/dlls/gdi32/enhmetafile.c @@ -768,10 +768,12 @@ static BOOL emr_produces_output(int type) } } -static HGDIOBJ get_object_handle(HANDLETABLE *handletable, DWORD i) +static HGDIOBJ get_object_handle(HANDLETABLE *handletable, UINT handles, DWORD i) { if (i & 0x80000000) return GetStockObject( i & 0x7fffffff ); + if (i >= handles) + return NULL; return handletable->objectHandle[i]; } @@ -905,12 +907,13 @@ BOOL WINAPI PlayEnhMetaFileRecord( case EMR_SELECTOBJECT: { const EMRSELECTOBJECT *pSelectObject = (const EMRSELECTOBJECT *)mr; - SelectObject( hdc, get_object_handle(handletable, pSelectObject->ihObject) ); + SelectObject( hdc, get_object_handle(handletable, handles, pSelectObject->ihObject) ); break; } case EMR_DELETEOBJECT: { const EMRDELETEOBJECT *pDeleteObject = (const EMRDELETEOBJECT *)mr; + if (pDeleteObject->ihObject >= handles) break; DeleteObject( (handletable->objectHandle)[pDeleteObject->ihObject]); (handletable->objectHandle)[pDeleteObject->ihObject] = 0; break; @@ -980,6 +983,7 @@ BOOL WINAPI PlayEnhMetaFileRecord( case EMR_CREATEPEN: { const EMRCREATEPEN *pCreatePen = (const EMRCREATEPEN *)mr; + if (pCreatePen->ihPen >= handles) break; (handletable->objectHandle)[pCreatePen->ihPen] = CreatePenIndirect(&pCreatePen->lopn); break; @@ -988,6 +992,9 @@ BOOL WINAPI PlayEnhMetaFileRecord( { const EMREXTCREATEPEN *pPen = (const EMREXTCREATEPEN *)mr; LOGBRUSH lb; + + if (pPen->ihPen >= handles) break; + lb.lbStyle = pPen->elp.elpBrushStyle; lb.lbColor = pPen->elp.elpColor; lb.lbHatch = pPen->elp.elpHatch; @@ -1004,6 +1011,9 @@ BOOL WINAPI PlayEnhMetaFileRecord( { const EMRCREATEBRUSHINDIRECT *pBrush = (const EMRCREATEBRUSHINDIRECT *)mr; LOGBRUSH brush; + + if (pBrush->ihBrush >= handles) break; + brush.lbStyle = pBrush->lb.lbStyle; brush.lbColor = pBrush->lb.lbColor; brush.lbHatch = pBrush->lb.lbHatch; @@ -1013,6 +1023,7 @@ BOOL WINAPI PlayEnhMetaFileRecord( case EMR_EXTCREATEFONTINDIRECTW: { const EMREXTCREATEFONTINDIRECTW *pFont = (const EMREXTCREATEFONTINDIRECTW *)mr; + if (pFont->ihFont >= handles) break; (handletable->objectHandle)[pFont->ihFont] = CreateFontIndirectW(&pFont->elfw.elfLogFont); break; @@ -1281,6 +1292,7 @@ BOOL WINAPI PlayEnhMetaFileRecord( { const EMRCREATEPALETTE *lpCreatePal = (const EMRCREATEPALETTE *)mr; + if (lpCreatePal->ihPal >= handles) break; (handletable->objectHandle)[ lpCreatePal->ihPal ] = CreatePalette( &lpCreatePal->lgpl ); @@ -1291,7 +1303,7 @@ BOOL WINAPI PlayEnhMetaFileRecord( { const EMRSELECTPALETTE *lpSelectPal = (const EMRSELECTPALETTE *)mr; - SelectPalette( hdc, get_object_handle(handletable, lpSelectPal->ihPal), TRUE ); + SelectPalette( hdc, get_object_handle(handletable, handles, lpSelectPal->ihPal), TRUE ); break; } @@ -1752,6 +1764,7 @@ BOOL WINAPI PlayEnhMetaFileRecord( case EMR_CREATECOLORSPACE: { PEMRCREATECOLORSPACE lpCreateColorSpace = (PEMRCREATECOLORSPACE)mr; + if (lpCreateColorSpace->ihCS >= handles) break; (handletable->objectHandle)[lpCreateColorSpace->ihCS] = CreateColorSpaceA( &lpCreateColorSpace->lcs ); break; @@ -1760,6 +1773,7 @@ BOOL WINAPI PlayEnhMetaFileRecord( case EMR_SETCOLORSPACE: { const EMRSETCOLORSPACE *lpSetColorSpace = (const EMRSETCOLORSPACE *)mr; + if (lpSetColorSpace->ihCS >= handles) break; SetColorSpace( hdc, (handletable->objectHandle)[lpSetColorSpace->ihCS] ); break; @@ -1768,6 +1782,7 @@ BOOL WINAPI PlayEnhMetaFileRecord( case EMR_DELETECOLORSPACE: { const EMRDELETECOLORSPACE *lpDeleteColorSpace = (const EMRDELETECOLORSPACE *)mr; + if (lpDeleteColorSpace->ihCS >= handles) break; DeleteColorSpace( (handletable->objectHandle)[lpDeleteColorSpace->ihCS] ); break; } @@ -1794,6 +1809,7 @@ BOOL WINAPI PlayEnhMetaFileRecord( { const EMRSETPALETTEENTRIES *lpSetPaletteEntries = (const EMRSETPALETTEENTRIES *)mr; + if (lpSetPaletteEntries->ihPal >= handles) break; SetPaletteEntries( (handletable->objectHandle)[lpSetPaletteEntries->ihPal], (UINT)lpSetPaletteEntries->iStart, (UINT)lpSetPaletteEntries->cEntries, @@ -1806,6 +1822,7 @@ BOOL WINAPI PlayEnhMetaFileRecord( { const EMRRESIZEPALETTE *lpResizePalette = (const EMRRESIZEPALETTE *)mr; + if (lpResizePalette->ihPal >= handles) break; NtGdiResizePalette( handletable->objectHandle[lpResizePalette->ihPal], lpResizePalette->cEntries ); @@ -1834,6 +1851,7 @@ BOOL WINAPI PlayEnhMetaFileRecord( break; } + if (lpCreate->ihBrush >= handles) break; (handletable->objectHandle)[lpCreate->ihBrush] = CreateDIBPatternBrushPt( (const BYTE *)lpCreate + lpCreate->offBmi, (UINT)lpCreate->iUsage ); @@ -1846,6 +1864,8 @@ BOOL WINAPI PlayEnhMetaFileRecord( const BITMAPINFO *pbi = (const BITMAPINFO *)((const BYTE *)mr + pCreateMonoBrush->offBmi); HBITMAP hBmp; + if (pCreateMonoBrush->ihBrush >= handles) break; + /* Need to check if the bitmap is monochrome, and if the two colors are really black and white */ if (pCreateMonoBrush->iUsage == DIB_PAL_INDICES || is_dib_monochrome(pbi)) @@ -2197,7 +2217,7 @@ BOOL WINAPI PlayEnhMetaFileRecord( { const EMRFILLRGN *pFillRgn = (const EMRFILLRGN *)mr; HRGN hRgn = ExtCreateRegion(NULL, pFillRgn->cbRgnData, (const RGNDATA *)pFillRgn->RgnData); - FillRgn(hdc, hRgn, get_object_handle(handletable, pFillRgn->ihBrush)); + FillRgn(hdc, hRgn, get_object_handle(handletable, handles, pFillRgn->ihBrush)); DeleteObject(hRgn); break; } @@ -2206,7 +2226,7 @@ BOOL WINAPI PlayEnhMetaFileRecord( { const EMRFRAMERGN *pFrameRgn = (const EMRFRAMERGN *)mr; HRGN hRgn = ExtCreateRegion(NULL, pFrameRgn->cbRgnData, (const RGNDATA *)pFrameRgn->RgnData); - FrameRgn(hdc, hRgn, get_object_handle(handletable, pFrameRgn->ihBrush), + FrameRgn(hdc, hRgn, get_object_handle(handletable, handles, pFrameRgn->ihBrush), pFrameRgn->szlStroke.cx, pFrameRgn->szlStroke.cy); DeleteObject(hRgn); break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6198
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=147649 Your paranoid android. === debian11b (64 bit WoW report) === ddraw: ddraw2.c:3735: Test failed: Expected screen size 1024x768, got 0x0. user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 0000000003DE0104, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032 Report validation errors: winmm:mci crashed (c0000008)
On Fri Aug 9 19:22:45 2024 +0000, Esme Povirk wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/6198/diffs?diff_id=125787&start_sha=741b88646e4d68a7aac882bed22ee6d4031429ac#dc91f1a1f8459a7d27c51aa4a84c2f0b09538f74_911_910) Updated.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6198#note_78221
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6198
participants (4)
-
Esme Povirk -
Esme Povirk (＠madewokherd)
-
Huw Davies (＠huw)
-
Marvin