[PATCH] ddraw: Validate the surface descriptor size in GetDisplayMode.

[Alexandre Julliard]

For bug 44180.

Signed-off-by: Alexandre Julliard 
---
 dlls/ddraw/ddraw.c        | 27 +++++++++++++--------------
 dlls/ddraw/tests/ddraw1.c | 17 +++++++++++++++++
 dlls/ddraw/tests/ddraw2.c | 17 +++++++++++++++++
 dlls/ddraw/tests/ddraw4.c | 17 +++++++++++++++++
 dlls/ddraw/tests/ddraw7.c | 17 +++++++++++++++++
 5 files changed, 81 insertions(+), 14 deletions(-)

diff --git a/dlls/ddraw/ddraw.c b/dlls/ddraw/ddraw.c
index d3e491c3e8bb..27be401a2e57 100644
--- a/dlls/ddraw/ddraw.c
+++ b/dlls/ddraw/ddraw.c
@@ -1627,17 +1627,14 @@ static HRESULT WINAPI ddraw7_GetDisplayMode(IDirectDraw7 *iface, DDSURFACEDESC2
     struct ddraw *ddraw = impl_from_IDirectDraw7(iface);
     struct wined3d_display_mode mode;
     HRESULT hr;
-    DWORD Size;

TRACE("iface %p, surface_desc %p.\n", iface, DDSD);

-    wined3d_mutex_lock();
     /* This seems sane */
-    if (!DDSD)
-    {
-        wined3d_mutex_unlock();
+    if (!DDSD || (DDSD->dwSize != sizeof(DDSURFACEDESC) && DDSD->dwSize != sizeof(DDSURFACEDESC2)))
         return DDERR_INVALIDPARAMS;
-    }
+
+    wined3d_mutex_lock();

if (FAILED(hr = wined3d_get_adapter_display_mode(ddraw->wined3d, WINED3DADAPTER_DEFAULT, &mode, NULL)))
     {
@@ -1646,10 +1643,8 @@ static HRESULT WINAPI ddraw7_GetDisplayMode(IDirectDraw7 *iface, DDSURFACEDESC2
         return hr;
     }

-    Size = DDSD->dwSize;
-    memset(DDSD, 0, Size);
-
-    DDSD->dwSize = Size;
+    memset(DDSD, 0, DDSD->dwSize);
+    DDSD->dwSize = sizeof(*DDSD);
     DDSD->dwFlags |= DDSD_HEIGHT | DDSD_WIDTH | DDSD_PIXELFORMAT | DDSD_PITCH | DDSD_REFRESHRATE;
     DDSD->dwWidth = mode.width;
     DDSD->dwHeight = mode.height;
@@ -1682,21 +1677,25 @@ static HRESULT WINAPI ddraw4_GetDisplayMode(IDirectDraw4 *iface, DDSURFACEDESC2
 static HRESULT WINAPI ddraw2_GetDisplayMode(IDirectDraw2 *iface, DDSURFACEDESC *surface_desc)
 {
     struct ddraw *ddraw = impl_from_IDirectDraw2(iface);
+    HRESULT hr;

TRACE("iface %p, surface_desc %p.\n", iface, surface_desc);

-    /* FIXME: Test sizes, properly convert surface_desc */
-    return ddraw7_GetDisplayMode(&ddraw->IDirectDraw7_iface, (DDSURFACEDESC2 *)surface_desc);
+    hr = ddraw7_GetDisplayMode(&ddraw->IDirectDraw7_iface, (DDSURFACEDESC2 *)surface_desc);
+    if (SUCCEEDED(hr)) surface_desc->dwSize = sizeof(*surface_desc);
+    return hr;
 }

static HRESULT WINAPI ddraw1_GetDisplayMode(IDirectDraw *iface, DDSURFACEDESC *surface_desc)
 {
     struct ddraw *ddraw = impl_from_IDirectDraw(iface);
+    HRESULT hr;

TRACE("iface %p, surface_desc %p.\n", iface, surface_desc);

-    /* FIXME: Test sizes, properly convert surface_desc */
-    return ddraw7_GetDisplayMode(&ddraw->IDirectDraw7_iface, (DDSURFACEDESC2 *)surface_desc);
+    hr = ddraw7_GetDisplayMode(&ddraw->IDirectDraw7_iface, (DDSURFACEDESC2 *)surface_desc);
+    if (SUCCEEDED(hr)) surface_desc->dwSize = sizeof(*surface_desc);
+    return hr;
 }

/*****************************************************************************
diff --git a/dlls/ddraw/tests/ddraw1.c b/dlls/ddraw/tests/ddraw1.c
index c306c4db1a32..b8ac74919f89 100644
--- a/dlls/ddraw/tests/ddraw1.c
+++ b/dlls/ddraw/tests/ddraw1.c
@@ -10132,6 +10132,23 @@ static void test_surface_desc_size(void)
         IDirectDrawSurface_Release(surface);
     }

+    /* GetDisplayMode() */
+    for (j = 0; j < sizeof(desc_sizes) / sizeof(*desc_sizes); ++j)
+    {
+        memset(&desc, 0xcc, sizeof(desc));
+        desc.dwSize = desc_sizes[j];
+        expected_hr = (desc.dwSize == sizeof(DDSURFACEDESC) || desc.dwSize == sizeof(DDSURFACEDESC2))
+                ? DD_OK : DDERR_INVALIDPARAMS;
+        hr = IDirectDraw_GetDisplayMode(ddraw, &desc.desc1);
+        ok(hr == expected_hr, "Got hr %#x, expected %#x, size %u.\n", hr, expected_hr, desc_sizes[j]);
+        if (SUCCEEDED(hr))
+        {
+            ok(desc.dwSize == sizeof(DDSURFACEDESC), "Wrong size %u for %u.\n", desc.dwSize, desc_sizes[j]);
+            ok(desc.blob[desc_sizes[j]] == 0xcc, "Overflow for size %u.\n", desc_sizes[j]);
+            ok(desc.blob[desc_sizes[j] - 1] != 0xcc, "Struct not cleared for size %u.\n", desc_sizes[j]);
+        }
+    }
+
     refcount = IDirectDraw7_Release(ddraw);
     ok(!refcount, "DirectDraw has %u references left.\n", refcount);
 }
diff --git a/dlls/ddraw/tests/ddraw2.c b/dlls/ddraw/tests/ddraw2.c
index 8a7801ee3e57..716103335f1b 100644
--- a/dlls/ddraw/tests/ddraw2.c
+++ b/dlls/ddraw/tests/ddraw2.c
@@ -11643,6 +11643,23 @@ static void test_surface_desc_size(void)
         IDirectDrawSurface_Release(surface);
     }

+    /* GetDisplayMode() */
+    for (j = 0; j < sizeof(desc_sizes) / sizeof(*desc_sizes); ++j)
+    {
+        memset(&desc, 0xcc, sizeof(desc));
+        desc.dwSize = desc_sizes[j];
+        expected_hr = (desc.dwSize == sizeof(DDSURFACEDESC) || desc.dwSize == sizeof(DDSURFACEDESC2))
+                ? DD_OK : DDERR_INVALIDPARAMS;
+        hr = IDirectDraw2_GetDisplayMode(ddraw, &desc.desc1);
+        ok(hr == expected_hr, "Got hr %#x, expected %#x, size %u.\n", hr, expected_hr, desc_sizes[j]);
+        if (SUCCEEDED(hr))
+        {
+            ok(desc.dwSize == sizeof(DDSURFACEDESC), "Wrong size %u for %u.\n", desc.dwSize, desc_sizes[j]);
+            ok(desc.blob[desc_sizes[j]] == 0xcc, "Overflow for size %u.\n", desc_sizes[j]);
+            ok(desc.blob[desc_sizes[j] - 1] != 0xcc, "Struct not cleared for size %u.\n", desc_sizes[j]);
+        }
+    }
+
     refcount = IDirectDraw2_Release(ddraw);
     ok(!refcount, "DirectDraw has %u references left.\n", refcount);
 }
diff --git a/dlls/ddraw/tests/ddraw4.c b/dlls/ddraw/tests/ddraw4.c
index 048331dfc39d..46362e5c1dc0 100644
--- a/dlls/ddraw/tests/ddraw4.c
+++ b/dlls/ddraw/tests/ddraw4.c
@@ -13247,6 +13247,23 @@ static void test_surface_desc_size(void)
         IDirectDrawSurface_Release(surface);
     }

+    /* GetDisplayMode() */
+    for (j = 0; j < sizeof(desc_sizes) / sizeof(*desc_sizes); ++j)
+    {
+        memset(&desc, 0xcc, sizeof(desc));
+        desc.dwSize = desc_sizes[j];
+        expected_hr = (desc.dwSize == sizeof(DDSURFACEDESC) || desc.dwSize == sizeof(DDSURFACEDESC2))
+                ? DD_OK : DDERR_INVALIDPARAMS;
+        hr = IDirectDraw4_GetDisplayMode(ddraw, &desc.desc2);
+        ok(hr == expected_hr, "Got hr %#x, expected %#x, size %u.\n", hr, expected_hr, desc_sizes[j]);
+        if (SUCCEEDED(hr))
+        {
+            ok(desc.dwSize == sizeof(DDSURFACEDESC2), "Wrong size %u for %u.\n", desc.dwSize, desc_sizes[j]);
+            ok(desc.blob[desc_sizes[j]] == 0xcc, "Overflow for size %u.\n", desc_sizes[j]);
+            ok(desc.blob[desc_sizes[j] - 1] != 0xcc, "Struct not cleared for size %u.\n", desc_sizes[j]);
+        }
+    }
+
     refcount = IDirectDraw4_Release(ddraw);
     ok(!refcount, "DirectDraw has %u references left.\n", refcount);
 }
diff --git a/dlls/ddraw/tests/ddraw7.c b/dlls/ddraw/tests/ddraw7.c
index d1ff85d5d54f..c7a2b138e97b 100644
--- a/dlls/ddraw/tests/ddraw7.c
+++ b/dlls/ddraw/tests/ddraw7.c
@@ -12547,6 +12547,23 @@ static void test_surface_desc_size(void)
         IDirectDrawSurface_Release(surface);
     }

+    /* GetDisplayMode() */
+    for (j = 0; j < sizeof(desc_sizes) / sizeof(*desc_sizes); ++j)
+    {
+        memset(&desc, 0xcc, sizeof(desc));
+        desc.dwSize = desc_sizes[j];
+        expected_hr = (desc.dwSize == sizeof(DDSURFACEDESC) || desc.dwSize == sizeof(DDSURFACEDESC2))
+                ? DD_OK : DDERR_INVALIDPARAMS;
+        hr = IDirectDraw7_GetDisplayMode(ddraw, &desc.desc2);
+        ok(hr == expected_hr, "Got hr %#x, expected %#x, size %u.\n", hr, expected_hr, desc_sizes[j]);
+        if (SUCCEEDED(hr))
+        {
+            ok(desc.dwSize == sizeof(DDSURFACEDESC2), "Wrong size %u for %u.\n", desc.dwSize, desc_sizes[j]);
+            ok(desc.blob[desc_sizes[j]] == 0xcc, "Overflow for size %u.\n", desc_sizes[j]);
+            ok(desc.blob[desc_sizes[j] - 1] != 0xcc, "Struct not cleared for size %u.\n", desc_sizes[j]);
+        }
+    }
+
     refcount = IDirectDraw7_Release(ddraw);
     ok(!refcount, "DirectDraw has %u references left.\n", refcount);
 }

[Henri Verbeet]

Signed-off-by: Henri Verbeet

[Henri Verbeet]

On 18 December 2017 at 11:12, Alexandre Julliard  wrote:
> diff --git a/dlls/ddraw/tests/ddraw1.c b/dlls/ddraw/tests/ddraw1.c
> index c306c4db1a32..b8ac74919f89 100644
> --- a/dlls/ddraw/tests/ddraw1.c
> +++ b/dlls/ddraw/tests/ddraw1.c
> @@ -10132,6 +10132,23 @@ static void test_surface_desc_size(void)
>          IDirectDrawSurface_Release(surface);
>      }
> - /* GetDisplayMode() */
> - for (j = 0; j < sizeof(desc_sizes) / sizeof(*desc_sizes); ++j)

We have an ARRAY_SIZE macro in the ddraw tests.