[PATCH 1/5] ddraw: Protect against invalid clipper pointers.

[Stefan Dösinger]

Signed-off-by: Stefan Dösinger 
---
 dlls/ddraw/clipper.c       | 66 +++++++++++++++++++++++++++++++++++---
 dlls/ddraw/ddraw_private.h |  1 +
 2 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/dlls/ddraw/clipper.c b/dlls/ddraw/clipper.c
index 01cac40ec6e..95036538d8f 100644
--- a/dlls/ddraw/clipper.c
+++ b/dlls/ddraw/clipper.c
@@ -26,9 +26,28 @@

WINE_DEFAULT_DEBUG_CHANNEL(ddraw);

+static const struct IDirectDrawClipperVtbl ddraw_clipper_vtbl;
+
+#define DDRAW_CLIPPER_MAGIC (MAKEFOURCC('C','L','I','P'))
+
 static inline struct ddraw_clipper *impl_from_IDirectDrawClipper(IDirectDrawClipper *iface)
 {
-    return CONTAINING_RECORD(iface, struct ddraw_clipper, IDirectDrawClipper_iface);
+    /* Native is very lenient when you invoke the methods (clipper or surface) with a clipper
+     * pointer that points to something that is either not accessible or not a clipper. Deus
+     * Ex: Goty depends on this. */
+    struct ddraw_clipper *clipper = CONTAINING_RECORD(iface, struct ddraw_clipper, IDirectDrawClipper_iface);
+
+    if (IsBadReadPtr(clipper, sizeof(*clipper)))
+    {
+        WARN("The application gave us a bad clipper.\n");
+        return NULL;
+    }
+    if (clipper->magic != DDRAW_CLIPPER_MAGIC)
+    {
+        WARN("The application gave us a bad clipper.\n");
+        return NULL;
+    }
+    return clipper;
 }

static HRESULT WINAPI ddraw_clipper_QueryInterface(IDirectDrawClipper *iface, REFIID iid, void **object)
@@ -37,6 +56,9 @@ static HRESULT WINAPI ddraw_clipper_QueryInterface(IDirectDrawClipper *iface, RE

TRACE("iface %p, iid %s, object %p.\n", iface, debugstr_guid(iid), object);

+    if (!clipper)
+        return DDERR_INVALIDPARAMS;
+
     if (IsEqualGUID(&IID_IDirectDrawClipper, iid)
             || IsEqualGUID(&IID_IUnknown, iid))
     {
@@ -54,17 +76,31 @@ static HRESULT WINAPI ddraw_clipper_QueryInterface(IDirectDrawClipper *iface, RE
 static ULONG WINAPI ddraw_clipper_AddRef(IDirectDrawClipper *iface)
 {
     struct ddraw_clipper *clipper = impl_from_IDirectDrawClipper(iface);
-    ULONG refcount = InterlockedIncrement(&clipper->ref);
+    ULONG refcount;

-    TRACE("%p increasing refcount to %u.\n", clipper, refcount);
+    if (!clipper)
+    {
+        WARN("Invalid clipper, returning 0.\n");
+        return 0;
+    }

+    refcount = InterlockedIncrement(&clipper->ref);
+    TRACE("%p increasing refcount to %u.\n", clipper, refcount);
     return refcount;
 }

static ULONG WINAPI ddraw_clipper_Release(IDirectDrawClipper *iface)
 {
     struct ddraw_clipper *clipper = impl_from_IDirectDrawClipper(iface);
-    ULONG refcount = InterlockedDecrement(&clipper->ref);
+    ULONG refcount;
+
+    if (!clipper || iface->lpVtbl != &ddraw_clipper_vtbl)
+    {
+        WARN("Invalid clipper, returning 0.\n");
+        return 0;
+    }
+
+    refcount = InterlockedDecrement(&clipper->ref);

TRACE("%p decreasing refcount to %u.\n", clipper, refcount);

@@ -72,6 +108,7 @@ static ULONG WINAPI ddraw_clipper_Release(IDirectDrawClipper *iface)
     {
         if (clipper->region)
             DeleteObject(clipper->region);
+        clipper->magic = 0; /* Should help with detecting freed clippers. */
         heap_free(clipper);
     }

@@ -84,6 +121,9 @@ static HRESULT WINAPI ddraw_clipper_SetHWnd(IDirectDrawClipper *iface, DWORD fla

TRACE("iface %p, flags %#x, window %p.\n", iface, flags, window);

+    if (!clipper)
+        return DDERR_INVALIDPARAMS;
+
     if (flags)
     {
         FIXME("flags %#x, not supported.\n", flags);
@@ -161,6 +201,9 @@ static HRESULT WINAPI ddraw_clipper_GetClipList(IDirectDrawClipper *iface, RECT
     TRACE("iface %p, rect %s, clip_list %p, clip_list_size %p.\n",
             iface, wine_dbgstr_rect(rect), clip_list, clip_list_size);

+    if (!clipper)
+        return DDERR_INVALIDPARAMS;
+
     wined3d_mutex_lock();

if (clipper->window)
@@ -238,6 +281,9 @@ static HRESULT WINAPI ddraw_clipper_SetClipList(IDirectDrawClipper *iface, RGNDA

TRACE("iface %p, region %p, flags %#x.\n", iface, region, flags);

+    if (!clipper)
+        return DDERR_INVALIDPARAMS;
+
     wined3d_mutex_lock();

if (clipper->window)
@@ -268,6 +314,9 @@ static HRESULT WINAPI ddraw_clipper_GetHWnd(IDirectDrawClipper *iface, HWND *win

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

+    if (!clipper)
+        return DDERR_INVALIDPARAMS;
+
     wined3d_mutex_lock();
     *window = clipper->window;
     wined3d_mutex_unlock();
@@ -282,6 +331,9 @@ static HRESULT WINAPI ddraw_clipper_Initialize(IDirectDrawClipper *iface,

TRACE("iface %p, ddraw %p, flags %#x.\n", iface, ddraw, flags);

+    if (!clipper)
+        return DDERR_INVALIDPARAMS;
+
     wined3d_mutex_lock();
     if (clipper->initialized)
     {
@@ -297,8 +349,13 @@ static HRESULT WINAPI ddraw_clipper_Initialize(IDirectDrawClipper *iface,

static HRESULT WINAPI ddraw_clipper_IsClipListChanged(IDirectDrawClipper *iface, BOOL *changed)
 {
+    struct ddraw_clipper *clipper = impl_from_IDirectDrawClipper(iface);
+
     FIXME("iface %p, changed %p stub!\n", iface, changed);

+    if (!clipper)
+        return DDERR_INVALIDPARAMS;
+
     /* XXX What is safest? */
     *changed = FALSE;

@@ -322,6 +379,7 @@ HRESULT ddraw_clipper_init(struct ddraw_clipper *clipper)
 {
     clipper->IDirectDrawClipper_iface.lpVtbl = &ddraw_clipper_vtbl;
     clipper->ref = 1;
+    clipper->magic = DDRAW_CLIPPER_MAGIC;

return DD_OK;
 }
diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h
index 77e28662724..e6971e9bc7f 100644
--- a/dlls/ddraw/ddraw_private.h
+++ b/dlls/ddraw/ddraw_private.h
@@ -387,6 +387,7 @@ struct ddraw_clipper
     HWND window;
     HRGN region;
     BOOL initialized;
+    DWORD magic;
 };

HRESULT ddraw_clipper_init(struct ddraw_clipper *clipper) DECLSPEC_HIDDEN;