[PATCH 0/1] MR6198: gdi32: Bounds check EMF handle tables.

[Esme Povirk (＠madewokherd)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45106

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6198

[PATCH 1/1] gdi32: Bounds check EMF handle tables.

[Esme Povirk]

From: Esme Povirk <esme(a)codeweavers.com>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45106
---
 dlls/gdi32/enhmetafile.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/dlls/gdi32/enhmetafile.c b/dlls/gdi32/enhmetafile.c
index 0ef26522eb6..680bbb5570c 100644
--- a/dlls/gdi32/enhmetafile.c
+++ b/dlls/gdi32/enhmetafile.c
@@ -556,6 +556,7 @@ typedef struct enum_emh_data
     EMF_dc_state state;
     INT save_level;
     EMF_dc_state *saved_state;
+    ENHMETAHEADER *header;
 } enum_emh_data;
 
 #define ENUM_GET_PRIVATE_DATA(ht) \
@@ -772,6 +773,8 @@ static HGDIOBJ get_object_handle(HANDLETABLE *handletable, DWORD i)
 {
     if (i & 0x80000000)
         return GetStockObject( i & 0x7fffffff );
+    if (i >= ENUM_GET_PRIVATE_DATA(handletable)->header->nHandles)
+        return NULL;
     return handletable->objectHandle[i];
 }
 
@@ -905,12 +908,14 @@ BOOL WINAPI PlayEnhMetaFileRecord(
     case EMR_SELECTOBJECT:
       {
 	const EMRSELECTOBJECT *pSelectObject = (const EMRSELECTOBJECT *)mr;
+	if (pSelectObject->ihObject >= info->header->nHandles) break;
 	SelectObject( hdc, get_object_handle(handletable, pSelectObject->ihObject) );
 	break;
       }
     case EMR_DELETEOBJECT:
       {
 	const EMRDELETEOBJECT *pDeleteObject = (const EMRDELETEOBJECT *)mr;
+	if (pDeleteObject->ihObject >= info->header->nHandles) break;
 	DeleteObject( (handletable->objectHandle)[pDeleteObject->ihObject]);
 	(handletable->objectHandle)[pDeleteObject->ihObject] = 0;
 	break;
@@ -980,6 +985,7 @@ BOOL WINAPI PlayEnhMetaFileRecord(
     case EMR_CREATEPEN:
       {
 	const EMRCREATEPEN *pCreatePen = (const EMRCREATEPEN *)mr;
+	if (pCreatePen->ihPen >= info->header->nHandles) break;
 	(handletable->objectHandle)[pCreatePen->ihPen] =
 	  CreatePenIndirect(&pCreatePen->lopn);
 	break;
@@ -988,6 +994,9 @@ BOOL WINAPI PlayEnhMetaFileRecord(
       {
 	const EMREXTCREATEPEN *pPen = (const EMREXTCREATEPEN *)mr;
 	LOGBRUSH lb;
+
+	if (pPen->ihPen >= info->header->nHandles) break;
+
 	lb.lbStyle = pPen->elp.elpBrushStyle;
 	lb.lbColor = pPen->elp.elpColor;
 	lb.lbHatch = pPen->elp.elpHatch;
@@ -1004,6 +1013,9 @@ BOOL WINAPI PlayEnhMetaFileRecord(
       {
 	const EMRCREATEBRUSHINDIRECT *pBrush = (const EMRCREATEBRUSHINDIRECT *)mr;
         LOGBRUSH brush;
+
+        if (pBrush->ihBrush >= info->header->nHandles) break;
+
         brush.lbStyle = pBrush->lb.lbStyle;
         brush.lbColor = pBrush->lb.lbColor;
         brush.lbHatch = pBrush->lb.lbHatch;
@@ -1013,6 +1025,7 @@ BOOL WINAPI PlayEnhMetaFileRecord(
     case EMR_EXTCREATEFONTINDIRECTW:
       {
 	const EMREXTCREATEFONTINDIRECTW *pFont = (const EMREXTCREATEFONTINDIRECTW *)mr;
+	if (pFont->ihFont >= info->header->nHandles) break;
 	(handletable->objectHandle)[pFont->ihFont] =
 	  CreateFontIndirectW(&pFont->elfw.elfLogFont);
 	break;
@@ -1281,6 +1294,7 @@ BOOL WINAPI PlayEnhMetaFileRecord(
       {
 	const EMRCREATEPALETTE *lpCreatePal = (const EMRCREATEPALETTE *)mr;
 
+	if (lpCreatePal->ihPal >= info->header->nHandles) break;
 	(handletable->objectHandle)[ lpCreatePal->ihPal ] =
 		CreatePalette( &lpCreatePal->lgpl );
 
@@ -1291,6 +1305,7 @@ BOOL WINAPI PlayEnhMetaFileRecord(
       {
 	const EMRSELECTPALETTE *lpSelectPal = (const EMRSELECTPALETTE *)mr;
 
+	if (lpSelectPal->ihPal >= info->header->nHandles) break;
 	SelectPalette( hdc, get_object_handle(handletable, lpSelectPal->ihPal), TRUE );
 	break;
       }
@@ -1752,6 +1767,7 @@ BOOL WINAPI PlayEnhMetaFileRecord(
     case EMR_CREATECOLORSPACE:
       {
         PEMRCREATECOLORSPACE lpCreateColorSpace = (PEMRCREATECOLORSPACE)mr;
+        if (lpCreateColorSpace->ihCS >= info->header->nHandles) break;
         (handletable->objectHandle)[lpCreateColorSpace->ihCS] =
            CreateColorSpaceA( &lpCreateColorSpace->lcs );
         break;
@@ -1760,6 +1776,7 @@ BOOL WINAPI PlayEnhMetaFileRecord(
     case EMR_SETCOLORSPACE:
       {
         const EMRSETCOLORSPACE *lpSetColorSpace = (const EMRSETCOLORSPACE *)mr;
+        if (lpSetColorSpace->ihCS >= info->header->nHandles) break;
         SetColorSpace( hdc,
                        (handletable->objectHandle)[lpSetColorSpace->ihCS] );
         break;
@@ -1768,6 +1785,7 @@ BOOL WINAPI PlayEnhMetaFileRecord(
     case EMR_DELETECOLORSPACE:
       {
         const EMRDELETECOLORSPACE *lpDeleteColorSpace = (const EMRDELETECOLORSPACE *)mr;
+        if (lpDeleteColorSpace->ihCS >= info->header->nHandles) break;
         DeleteColorSpace( (handletable->objectHandle)[lpDeleteColorSpace->ihCS] );
         break;
       }
@@ -1794,6 +1812,7 @@ BOOL WINAPI PlayEnhMetaFileRecord(
       {
         const EMRSETPALETTEENTRIES *lpSetPaletteEntries = (const EMRSETPALETTEENTRIES *)mr;
 
+        if (lpSetPaletteEntries->ihPal >= info->header->nHandles) break;
         SetPaletteEntries( (handletable->objectHandle)[lpSetPaletteEntries->ihPal],
                            (UINT)lpSetPaletteEntries->iStart,
                            (UINT)lpSetPaletteEntries->cEntries,
@@ -1806,6 +1825,7 @@ BOOL WINAPI PlayEnhMetaFileRecord(
       {
         const EMRRESIZEPALETTE *lpResizePalette = (const EMRRESIZEPALETTE *)mr;
 
+        if (lpResizePalette->ihPal >= info->header->nHandles) break;
         NtGdiResizePalette( handletable->objectHandle[lpResizePalette->ihPal],
                             lpResizePalette->cEntries );
 
@@ -1834,6 +1854,7 @@ BOOL WINAPI PlayEnhMetaFileRecord(
             break;
         }
 
+        if (lpCreate->ihBrush >= info->header->nHandles) break;
         (handletable->objectHandle)[lpCreate->ihBrush] =
            CreateDIBPatternBrushPt( (const BYTE *)lpCreate + lpCreate->offBmi,
                                     (UINT)lpCreate->iUsage );
@@ -1846,6 +1867,8 @@ BOOL WINAPI PlayEnhMetaFileRecord(
         const BITMAPINFO *pbi = (const BITMAPINFO *)((const BYTE *)mr + pCreateMonoBrush->offBmi);
         HBITMAP hBmp;
 
+        if (pCreateMonoBrush->ihBrush >= info->header->nHandles) 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))
@@ -2425,6 +2448,7 @@ BOOL WINAPI EnumEnhMetaFile(
     info->save_level = 0;
     info->saved_state = NULL;
     info->init_transform = info->state.world_transform;
+    info->header = emh;
 
     ht = (HANDLETABLE*) &info[1];
     ht->objectHandle[0] = hmf;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/6198

[PATCH 1/1] gdi32: Bounds check EMF handle tables.

[Marvin]

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=147532

Your paranoid android.


=== debian11b (64 bit WoW report) ===

ddraw:
ddraw4.c:3969: Test failed: Expected message 0x5, but didn't receive it.

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/gdi32/enhmetafile.c:
> case EMR_SELECTOBJECT:
>        {
>   const EMRSELECTOBJECT *pSelectObject = (const EMRSELECTOBJECT *)mr;
> +	if (pSelectObject->ihObject >= info->header->nHandles) break;

Can't we compare to `handles` instead?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6198#note_78089

[Esme Povirk (＠madewokherd)]

On Thu Aug  8 10:53:54 2024 +0000, Huw Davies wrote:
> Can't we compare to `handles` instead?
> Oh, I guess so.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6198#note_78217