[PATCH 0/3] MR1292: uxtheme: Protect CloseThemeData() from invalid input.

[Zhiyi Zhang (＠zhiyi)]

The first patch is from Dmitry. I asked him before sending it.

[PATCH 1/3] uxtheme: Protect CloseThemeData() from invalid input.

[Dmitry Timoshkov]

From: Dmitry Timoshkov

From: Dmitry Timoshkov

With test case by Michael Müller .

Zhiyi Zhang's comments:

Some applications close the same HTHEME handle more than once, causing use-after-free. HTHEME is a
handle rather than a pointer. Some testing shows that it's a handle starting from 0x10000 or 0x20000.
Each new handle increments from the first handle and closing handles decrements it. I prefer not to
implement this handle to data map for now because it will likely hurt performance.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=29974
---
 dlls/uxtheme/msstyles.c     | 22 ++++++++++++++++++++++
 dlls/uxtheme/msstyles.h     |  1 +
 dlls/uxtheme/tests/system.c |  2 ++
 3 files changed, 25 insertions(+)

diff --git a/dlls/uxtheme/msstyles.c b/dlls/uxtheme/msstyles.c
index 9e5a79a1c69..d871f213b05 100644
--- a/dlls/uxtheme/msstyles.c
+++ b/dlls/uxtheme/msstyles.c
@@ -32,6 +32,7 @@

#include "msstyles.h"

+#include "wine/exception.h"
 #include "wine/debug.h"
 #include "wine/heap.h"

@@ -49,6 +50,8 @@ static HRESULT MSSTYLES_GetFont (LPCWSTR lpStringStart, LPCWSTR lpStringEnd, LPC

#define MSSTYLES_VERSION 0x0003

+#define THEME_CLASS_SIGNATURE 0x12bc6d83
+
 static PTHEME_FILE tfActiveTheme;

/***********************************************************************/
@@ -204,6 +207,7 @@ void MSSTYLES_CloseThemeFile(PTHEME_FILE tf)
                         pcls->partstate = ps->next;
                         heap_free(ps);
                     }
+                    pcls->signature = 0;
                     heap_free(pcls);
                 }
             }
@@ -442,6 +446,7 @@ static PTHEME_CLASS MSSTYLES_AddClass(PTHEME_FILE tf, LPCWSTR pszAppName, LPCWST
     if(cur) return cur;

cur = heap_alloc(sizeof(*cur));
+    cur->signature = THEME_CLASS_SIGNATURE;
     cur->hTheme = tf->hTheme;
     lstrcpyW(cur->szAppName, pszAppName);
     lstrcpyW(cur->szClassName, pszClassName);
@@ -1075,6 +1080,23 @@ PTHEME_CLASS MSSTYLES_OpenThemeClass(LPCWSTR pszAppName, LPCWSTR pszClassList, U
  */
 HRESULT MSSTYLES_CloseThemeClass(PTHEME_CLASS tc)
 {
+    __TRY
+    {
+        if (tc->signature != THEME_CLASS_SIGNATURE)
+            tc = NULL;
+    }
+    __EXCEPT_PAGE_FAULT
+    {
+        tc = NULL;
+    }
+    __ENDTRY
+
+    if (!tc)
+    {
+        WARN("Invalid theme class handle\n");
+        return E_HANDLE;
+    }
+
     MSSTYLES_CloseThemeFile (tc->tf);
     return S_OK;
 }
diff --git a/dlls/uxtheme/msstyles.h b/dlls/uxtheme/msstyles.h
index 67f81315d7a..a1918314341 100644
--- a/dlls/uxtheme/msstyles.h
+++ b/dlls/uxtheme/msstyles.h
@@ -49,6 +49,7 @@ typedef struct _THEME_PARTSTATE {
 struct _THEME_FILE;

typedef struct _THEME_CLASS {
+    DWORD signature;
     HMODULE hTheme;
     struct _THEME_FILE* tf;
     WCHAR szAppName[MAX_THEME_APP_NAME];
diff --git a/dlls/uxtheme/tests/system.c b/dlls/uxtheme/tests/system.c
index a90c512091f..f8253c6a983 100644
--- a/dlls/uxtheme/tests/system.c
+++ b/dlls/uxtheme/tests/system.c
@@ -835,6 +835,8 @@ static void test_CloseThemeData(void)
     ok( hRes == E_HANDLE, "Expected E_HANDLE, got 0x%08lx\n", hRes);
     hRes = CloseThemeData(INVALID_HANDLE_VALUE);
     ok( hRes == E_HANDLE, "Expected E_HANDLE, got 0x%08lx\n", hRes);
+    hRes = CloseThemeData((HTHEME)0xdeadbeef);
+    ok(hRes == E_HANDLE, "Expected E_HANDLE, got 0x%08lx\n", hRes);
 }

static void test_buffer_dc_props(HDC hdc, const RECT *rect)

[PATCH 2/3] uxtheme: Reference count theme classes.

[Zhiyi Zhang]

From: Zhiyi Zhang

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=29974
Signed-off-by: Zhiyi Zhang 
---
 dlls/uxtheme/msstyles.c | 9 ++++++++-
 dlls/uxtheme/msstyles.h | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/dlls/uxtheme/msstyles.c b/dlls/uxtheme/msstyles.c
index d871f213b05..529a280b2e5 100644
--- a/dlls/uxtheme/msstyles.c
+++ b/dlls/uxtheme/msstyles.c
@@ -447,6 +447,7 @@ static PTHEME_CLASS MSSTYLES_AddClass(PTHEME_FILE tf, LPCWSTR pszAppName, LPCWST

cur = heap_alloc(sizeof(*cur));
     cur->signature = THEME_CLASS_SIGNATURE;
+    cur->refcount = 0;
     cur->hTheme = tf->hTheme;
     lstrcpyW(cur->szAppName, pszAppName);
     lstrcpyW(cur->szClassName, pszClassName);
@@ -1061,6 +1062,7 @@ PTHEME_CLASS MSSTYLES_OpenThemeClass(LPCWSTR pszAppName, LPCWSTR pszClassList, U
         TRACE("Opened app %s, class %s from list %s\n", debugstr_w(cls->szAppName), debugstr_w(cls->szClassName), debugstr_w(pszClassList));
    cls->tf = tfActiveTheme;
    cls->tf->dwRefCount++;
+        InterlockedIncrement(&cls->refcount);
         cls->dpi = dpi;
     }
     return cls;
@@ -1080,6 +1082,8 @@ PTHEME_CLASS MSSTYLES_OpenThemeClass(LPCWSTR pszAppName, LPCWSTR pszClassList, U
  */
 HRESULT MSSTYLES_CloseThemeClass(PTHEME_CLASS tc)
 {
+    LONG refcount;
+
     __TRY
     {
         if (tc->signature != THEME_CLASS_SIGNATURE)
@@ -1097,7 +1101,10 @@ HRESULT MSSTYLES_CloseThemeClass(PTHEME_CLASS tc)
         return E_HANDLE;
     }

-    MSSTYLES_CloseThemeFile (tc->tf);
+    refcount = InterlockedDecrement(&tc->refcount);
+    /* Some buggy apps may double free HTHEME handles */
+    if (refcount >= 0)
+        MSSTYLES_CloseThemeFile(tc->tf);
     return S_OK;
 }

diff --git a/dlls/uxtheme/msstyles.h b/dlls/uxtheme/msstyles.h
index a1918314341..9fe99784838 100644
--- a/dlls/uxtheme/msstyles.h
+++ b/dlls/uxtheme/msstyles.h
@@ -50,6 +50,7 @@ struct _THEME_FILE;

typedef struct _THEME_CLASS {
     DWORD signature;
+    LONG refcount;
     HMODULE hTheme;
     struct _THEME_FILE* tf;
     WCHAR szAppName[MAX_THEME_APP_NAME];

[PATCH 3/3] uxtheme: Use atomic functions for reference counting.

[Zhiyi Zhang]

From: Zhiyi Zhang

Signed-off-by: Zhiyi Zhang 
---
 dlls/uxtheme/msstyles.c | 13 ++++++++-----
 dlls/uxtheme/msstyles.h |  2 +-
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/dlls/uxtheme/msstyles.c b/dlls/uxtheme/msstyles.c
index 529a280b2e5..cfd21a48989 100644
--- a/dlls/uxtheme/msstyles.c
+++ b/dlls/uxtheme/msstyles.c
@@ -171,7 +171,7 @@ HRESULT MSSTYLES_OpenThemeFile(LPCWSTR lpThemeFile, LPCWSTR pszColorName, LPCWST
     (*tf)->pszAvailSizes = pszSizes;
     (*tf)->pszSelectedColor = pszSelectedColor;
     (*tf)->pszSelectedSize = pszSelectedSize;
-    (*tf)->dwRefCount = 1;
+    (*tf)->refcount = 1;
     return S_OK;

invalid_theme:
@@ -187,9 +187,12 @@ invalid_theme:
  */
 void MSSTYLES_CloseThemeFile(PTHEME_FILE tf)
 {
+    LONG refcount;
+
     if(tf) {
-        tf->dwRefCount--;
-        if(!tf->dwRefCount) {
+        refcount = InterlockedDecrement(&tf->refcount);
+        if (!refcount)
+        {
             if(tf->hTheme) FreeLibrary(tf->hTheme);
             if(tf->classes) {
                 while(tf->classes) {
@@ -235,7 +238,7 @@ HRESULT MSSTYLES_SetActiveTheme(PTHEME_FILE tf, BOOL setMetrics)
     tfActiveTheme = tf;
     if (tfActiveTheme)
     {
-	tfActiveTheme->dwRefCount++;
+        InterlockedIncrement(&tfActiveTheme->refcount);
    if(!tfActiveTheme->classes)
        MSSTYLES_ParseThemeIni(tfActiveTheme, setMetrics);
     }
@@ -1061,7 +1064,7 @@ PTHEME_CLASS MSSTYLES_OpenThemeClass(LPCWSTR pszAppName, LPCWSTR pszClassList, U
     if(cls) {
         TRACE("Opened app %s, class %s from list %s\n", debugstr_w(cls->szAppName), debugstr_w(cls->szClassName), debugstr_w(pszClassList));
    cls->tf = tfActiveTheme;
-	cls->tf->dwRefCount++;
+        InterlockedIncrement(&cls->tf->refcount);
         InterlockedIncrement(&cls->refcount);
         cls->dpi = dpi;
     }
diff --git a/dlls/uxtheme/msstyles.h b/dlls/uxtheme/msstyles.h
index 9fe99784838..98e1aa045d6 100644
--- a/dlls/uxtheme/msstyles.h
+++ b/dlls/uxtheme/msstyles.h
@@ -71,7 +71,7 @@ typedef struct _THEME_IMAGE {
 } THEME_IMAGE, *PTHEME_IMAGE;

typedef struct _THEME_FILE {
-    DWORD dwRefCount;
+    LONG refcount;
     HMODULE hTheme;
     WCHAR szThemeFile[MAX_PATH];
     LPWSTR pszAvailColors;

[PATCH 0/3] MR1292: uxtheme: Protect CloseThemeData() from invalid input. - approved

[Zhiyi Zhang (＠zhiyi)]

This merge request was approved by Zhiyi Zhang.