[PATCH v2 0/1] MR8141: gdiplus: Add check of type parameter to be positive (ASan).
Function GdipGetImageAttributesAdjustedPalette gets [called with type = -1](https://gitlab.winehq.org/wine/wine/-/blob/22af42ac22279e6c0c671f033661f95c1...) and therefore accesses several arrays at index -1. This patch adds a helper which additionally checks for `type >= ColorAdjustTypeDefault`. <details> <summary>ASan output</summary> ``` ================================================================= ==gdiplus_test.exe==308==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f21342c57f8 at pc 0x6ffffadfd25b bp 0x7ffffe1ff750 sp 0x7ffffe1ff798 READ of size 4 at 0x7f21342c57f8 thread T0 #0 0x6ffffadfd25a in apply_image_attributes .../dlls/gdiplus/graphics.c:929:41 #1 0x6ffffae5c2f2 in GdipGetImageAttributesAdjustedPalette .../dlls/gdiplus/imageattributes.c:123:5 #2 0x000140084b59 in test_getadjustedpalette .../dlls/gdiplus/tests/image.c:5649:12 #3 0x000140062be6 in func_image .../dlls/gdiplus/tests/image.c:6719:5 #4 0x0001400c9132 in run_test .../include/wine/test.h:765:5 #5 0x0001400c8b50 in main .../include/wine/test.h:884:12 #6 0x0001400cad3a in mainCRTStartup .../dlls/msvcrt/crt_main.c:60:11 #7 0x6fffffc45aa0 in BaseThreadInitThunk .../dlls/kernel32/thread.c:61:24 #8 0x6fffffdcc776 in RtlUserThreadStart (C:\windows\system32\ntdll.dll+0x17004c776) 0x7f21342c57f8 is located 144 bytes after 1256-byte region [0x7f21342c5280,0x7f21342c5768) freed by thread T0 here: #0 0x6ffffe88aab1 in free C:/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan\asan_malloc_win.cpp:71:3 #1 0x6ffffae5c21d in GdipDisposeImageAttributes .../dlls/gdiplus/imageattributes.c:109:5 #2 0x000140083598 in test_colorkey .../dlls/gdiplus/tests/image.c:3709:5 #3 0x000140062bd7 in func_image .../dlls/gdiplus/tests/image.c:6716:5 #4 0x0001400c9132 in run_test .../include/wine/test.h:765:5 #5 0x0001400c8b50 in main .../include/wine/test.h:884:12 #6 0x0001400cad3a in mainCRTStartup .../dlls/msvcrt/crt_main.c:60:11 #7 0x6fffffc45aa0 in BaseThreadInitThunk .../dlls/kernel32/thread.c:61:24 #8 0x6fffffdcc776 in RtlUserThreadStart (C:\windows\system32\ntdll.dll+0x17004c776) previously allocated by thread T0 here: #0 0x6ffffe88ace6 in calloc C:/llvm-mingw/llvm-mingw/llvm-project/compiler-rt/lib/asan\asan_malloc_win.cpp:91:3 #1 0x6ffffae5c0fc in GdipCreateImageAttributes .../dlls/gdiplus/imageattributes.c:87:18 #2 0x000140082bf9 in test_colorkey .../dlls/gdiplus/tests/image.c:3629:12 #3 0x000140062bd7 in func_image .../dlls/gdiplus/tests/image.c:6716:5 #4 0x0001400c9132 in run_test .../include/wine/test.h:765:5 #5 0x0001400c8b50 in main .../include/wine/test.h:884:12 #6 0x0001400cad3a in mainCRTStartup .../dlls/msvcrt/crt_main.c:60:11 #7 0x6fffffc45aa0 in BaseThreadInitThunk .../dlls/kernel32/thread.c:61:24 #8 0x6fffffdcc776 in RtlUserThreadStart (C:\windows\system32\ntdll.dll+0x17004c776) SUMMARY: AddressSanitizer: heap-buffer-overflow .../dlls/gdiplus/graphics.c:929:41 in apply_image_attributes Shadow bytes around the buggy address: 0x7f21342c5500: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x7f21342c5580: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x7f21342c5600: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x7f21342c5680: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x7f21342c5700: fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa =>0x7f21342c5780: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa[fa] 0x7f21342c5800: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x7f21342c5880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x7f21342c5900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x7f21342c5980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x7f21342c5a00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==gdiplus_test.exe==308==ABORTING ``` </details> -- v2: gdiplus: Add check of type parameter to be positive (ASan). https://gitlab.winehq.org/wine/wine/-/merge_requests/8141
From: Bernhard Übelacker <bernhardu(a)mailbox.org> --- dlls/gdiplus/imageattributes.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/dlls/gdiplus/imageattributes.c b/dlls/gdiplus/imageattributes.c index 349e52f5be9..e19687db39b 100644 --- a/dlls/gdiplus/imageattributes.c +++ b/dlls/gdiplus/imageattributes.c @@ -27,6 +27,13 @@ WINE_DEFAULT_DEBUG_CHANNEL(gdiplus); +static inline int is_valid_ColorAdjustType(ColorAdjustType type) +{ + if (type >= ColorAdjustTypeDefault && type < ColorAdjustTypeCount) + return TRUE; + return FALSE; +} + GpStatus WINGDIPAPI GdipCloneImageAttributes(GDIPCONST GpImageAttributes *imageattr, GpImageAttributes **cloneImageattr) { @@ -117,7 +124,7 @@ GpStatus WINGDIPAPI GdipGetImageAttributesAdjustedPalette(GpImageAttributes *ima TRACE("(%p,%p,%u)\n", imageattr, palette, type); if (!imageattr || !palette || !palette->Count || - type >= ColorAdjustTypeCount || type == ColorAdjustTypeDefault) + !is_valid_ColorAdjustType(type) || type == ColorAdjustTypeDefault) return InvalidParameter; apply_image_attributes(imageattr, (LPBYTE)palette->Entries, palette->Count, 1, 0, @@ -131,7 +138,7 @@ GpStatus WINGDIPAPI GdipSetImageAttributesColorKeys(GpImageAttributes *imageattr { TRACE("(%p,%u,%i,%08lx,%08lx)\n", imageattr, type, enableFlag, colorLow, colorHigh); - if(!imageattr || type >= ColorAdjustTypeCount) + if(!imageattr || !is_valid_ColorAdjustType(type)) return InvalidParameter; imageattr->colorkeys[type].enabled = enableFlag; @@ -148,7 +155,7 @@ GpStatus WINGDIPAPI GdipSetImageAttributesColorMatrix(GpImageAttributes *imageat TRACE("(%p,%u,%i,%p,%p,%u)\n", imageattr, type, enableFlag, colorMatrix, grayMatrix, flags); - if(!imageattr || type >= ColorAdjustTypeCount || flags > ColorMatrixFlagsAltGray) + if(!imageattr || !is_valid_ColorAdjustType(type) || flags > ColorMatrixFlagsAltGray) return InvalidParameter; if (enableFlag) @@ -206,7 +213,7 @@ GpStatus WINGDIPAPI GdipSetImageAttributesGamma(GpImageAttributes *imageAttr, { TRACE("(%p,%u,%i,%0.2f)\n", imageAttr, type, enableFlag, gamma); - if (!imageAttr || (enableFlag && gamma <= 0.0) || type >= ColorAdjustTypeCount) + if (!imageAttr || (enableFlag && gamma <= 0.0) || !is_valid_ColorAdjustType(type)) return InvalidParameter; imageAttr->gamma_enabled[type] = enableFlag; @@ -220,7 +227,7 @@ GpStatus WINGDIPAPI GdipSetImageAttributesNoOp(GpImageAttributes *imageAttr, { TRACE("(%p,%u,%i)\n", imageAttr, type, enableFlag); - if (type >= ColorAdjustTypeCount) + if (!is_valid_ColorAdjustType(type)) return InvalidParameter; imageAttr->noop[type] = enableFlag ? IMAGEATTR_NOOP_SET : IMAGEATTR_NOOP_CLEAR; @@ -263,7 +270,7 @@ GpStatus WINGDIPAPI GdipSetImageAttributesRemapTable(GpImageAttributes *imageAtt TRACE("(%p,%u,%i,%u,%p)\n", imageAttr, type, enableFlag, mapSize, map); - if(!imageAttr || type >= ColorAdjustTypeCount) + if(!imageAttr || !is_valid_ColorAdjustType(type)) return InvalidParameter; if (enableFlag) @@ -325,7 +332,7 @@ GpStatus WINGDIPAPI GdipResetImageAttributes(GpImageAttributes *imageAttr, { TRACE("(%p,%u)\n", imageAttr, type); - if(!imageAttr || type >= ColorAdjustTypeCount) + if(!imageAttr || !is_valid_ColorAdjustType(type)) return InvalidParameter; memset(&imageAttr->colormatrices[type], 0, sizeof(imageAttr->colormatrices[type])); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8141
v2: - Removed wrong check from internal function apply_image_attributes. - Moved helper from header to imageattributes.c. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8141#note_104497
On Mon May 26 01:46:44 2025 +0000, Bernhard Übelacker wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/8141/diffs?diff_id=180605&start_sha=f1eb9919d0a4fd9bddecb8a2770b416cbf6f1bd8#aaf93e29527c1cbc47ec54679da5828ebd679128_859_857) Thanks for having a look and sorry for the wrong return type. I pushed a new version v2.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8141#note_104498
This merge request was approved by Esme Povirk. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8141
participants (2)
-
Bernhard Übelacker -
Esme Povirk (＠madewokherd)