From 749e280df7f0c121ed48740341f125a903099bb4 Mon Sep 17 00:00:00 2001 From: Alexandre Julliard Date: Mon, 18 Dec 2017 11:12:04 +0100 Subject: [PATCH] ddraw: Validate the surface descriptor size in GetDisplayMode. Signed-off-by: Alexandre Julliard Signed-off-by: Henri Verbeet 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 d3e491c3e8b..27be401a2e5 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 c306c4db1a3..109910b8205 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 < ARRAY_SIZE(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 8a7801ee3e5..1db7ad9329f 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 < ARRAY_SIZE(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 048331dfc39..7b7f1c2c218 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 < ARRAY_SIZE(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 d1ff85d5d54..c7a2b138e97 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); }