[PATCH v6 0/1] MR10929: gdi32: Validate count parameter in ExtCreateRegion

[Bartosz Kosiorek (＠gang65)]

--
  v6: gdi32: Validate count parameter in ExtCreateRegion
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10929

[PATCH v6 1/1] gdi32: Validate count parameter in ExtCreateRegion

[Bartosz Kosiorek]

From: Bartosz Kosiorek <[e-mail]>

---
 dlls/ddraw/clipper.c        | 2 +-
 dlls/gdi32/enhmetafile.c    | 2 +-
 dlls/gdi32/objects.c        | 2 +-
 dlls/gdi32/tests/clipping.c | 4 +---
 dlls/win32u/region.c        | 3 ++-
 5 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/dlls/ddraw/clipper.c b/dlls/ddraw/clipper.c
index 3159a34cb5a..2b13b0ee766 100644
--- a/dlls/ddraw/clipper.c
+++ b/dlls/ddraw/clipper.c
@@ -299,7 +299,7 @@ static HRESULT WINAPI ddraw_clipper_SetClipList(IDirectDrawClipper *iface, RGNDA
         DeleteObject(clipper->region);
     if (!region)
         clipper->region = NULL;
-    else if (!(clipper->region = ExtCreateRegion(NULL, 0, region)))
+    else if (!(clipper->region = ExtCreateRegion(NULL, region->rdh.dwSize + region->rdh.nRgnSize, region)))
     {
         wined3d_mutex_unlock();
         ERR("Failed to create region.\n");
diff --git a/dlls/gdi32/enhmetafile.c b/dlls/gdi32/enhmetafile.c
index fef9581e408..0f65d7c0194 100644
--- a/dlls/gdi32/enhmetafile.c
+++ b/dlls/gdi32/enhmetafile.c
@@ -1319,7 +1319,7 @@ BOOL WINAPI PlayEnhMetaFileRecord(
 	HRGN hRgn = 0;
 
         if (mr->nSize >= sizeof(*lpRgn) + sizeof(RGNDATAHEADER))
-            hRgn = ExtCreateRegion( &info->init_transform, 0, (const RGNDATA *)lpRgn->RgnData );
+            hRgn = ExtCreateRegion( &info->init_transform, lpRgn->cbRgnData, (const RGNDATA *)lpRgn->RgnData );
 
 	ExtSelectClipRgn(hdc, hRgn, (INT)(lpRgn->iMode));
 	/* ExtSelectClipRgn created a copy of the region */
diff --git a/dlls/gdi32/objects.c b/dlls/gdi32/objects.c
index f2b353cff27..12eaddcd623 100644
--- a/dlls/gdi32/objects.c
+++ b/dlls/gdi32/objects.c
@@ -642,7 +642,7 @@ HBITMAP WINAPI CreateDiscardableBitmap( HDC hdc, INT width, INT height )
  */
 HRGN WINAPI ExtCreateRegion( const XFORM *xform, DWORD count, const RGNDATA *data )
 {
-    if (!data)
+    if (!data || count < sizeof(RGNDATAHEADER))
     {
         SetLastError( ERROR_INVALID_PARAMETER );
         return 0;
diff --git a/dlls/gdi32/tests/clipping.c b/dlls/gdi32/tests/clipping.c
index ce66f9cad09..0ae467dbfdc 100644
--- a/dlls/gdi32/tests/clipping.c
+++ b/dlls/gdi32/tests/clipping.c
@@ -219,15 +219,14 @@ static void test_ExtCreateRegion(void)
     SetLastError(0xdeadbeef);
     hrgn = ExtCreateRegion(NULL, sizeof(RGNDATAHEADER), &rgn.data);
     ok(hrgn != 0, "ExtCreateRegion error %lu\n", GetLastError());
+    ok(GetLastError() == 0xdeadbeef, "0xdeadbeef, got %lu\n", GetLastError());
     verify_region(hrgn, &empty_rect);
     DeleteObject(hrgn);
 
     /* Cannot be smaller than sizeof(RGNDATAHEADER) */
     SetLastError(0xdeadbeef);
     hrgn = ExtCreateRegion(NULL, sizeof(RGNDATAHEADER) - 1, &rgn.data);
-    todo_wine
     ok(!hrgn, "ExtCreateRegion should fail\n");
-    todo_wine
     ok(GetLastError() == ERROR_INVALID_PARAMETER ||
        broken(GetLastError() == 0xdeadbeef), "0xdeadbeef, got %lu\n", GetLastError());
 
@@ -267,7 +266,6 @@ static void test_ExtCreateRegion(void)
     /* Buffer cannot be smaller than sizeof(RGNDATAHEADER) + 2 * sizeof(RECT) */
     SetLastError(0xdeadbeef);
     hrgn = ExtCreateRegion(NULL, sizeof(RGNDATAHEADER) + 2 * sizeof(RECT) - 1, &rgn.data);
-    todo_wine
     ok(!hrgn, "ExtCreateRegion should fail\n");
     ok(GetLastError() == 0xdeadbeef, "0xdeadbeef, got %lu\n", GetLastError());
 
diff --git a/dlls/win32u/region.c b/dlls/win32u/region.c
index 35ef4a22256..3c1a8bc0953 100644
--- a/dlls/win32u/region.c
+++ b/dlls/win32u/region.c
@@ -896,7 +896,8 @@ HRGN WINAPI NtGdiExtCreateRegion( const XFORM *xform, DWORD count, const RGNDATA
     WINEREGION *obj;
     const RECT *pCurRect, *pEndRect;
 
-    if (!rgndata || rgndata->rdh.dwSize < sizeof(RGNDATAHEADER))
+    if (!rgndata || rgndata->rdh.dwSize < sizeof(RGNDATAHEADER) ||
+        count < sizeof(RGNDATAHEADER) + rgndata->rdh.nCount * sizeof(RECT))
         return 0;
 
     /* XP doesn't care about the type */
-- 
GitLab

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

[Huw Davies (＠huw)]

Does anything actually depend on this or are you just 'fixing' todos in the tests?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10929#note_140552

[Bartosz Kosiorek (＠gang65)]

On Tue May 19 09:39:16 2026 +0000, Huw Davies wrote:
> Does anything actually depend on this or are you just 'fixing' todos in
> the tests?
> I am fixing the todos in tests. I don't have application which is using it. I think it will be safer to have such check.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10929#note_140583

[Bartosz Kosiorek (＠gang65)]

On Tue May 19 11:16:43 2026 +0000, Bartosz Kosiorek wrote:
> I am fixing the todos in tests. I don't have application which is using
> it. I think it will be safer to have such check.
> Do you think it will be a problem with merging it?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10929#note_140636

[Huw Davies (＠huw)]

On Tue May 19 19:43:47 2026 +0000, Bartosz Kosiorek wrote:
> Do you think it will be a problem with merging it?
> Fixing todos in tests generally isn't really worthwhile, unless those changes actually fix an application. Adding changes to Wine always carries a risk of breaking something and takes up both the author's and the reviewer's time.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10929#note_141032