shell32: handle invalid cbSize in Shell_NotifyIcon[AW] (3rd try)

Mikołaj Zalewski mikolaj at zalewski.pl
Wed May 9 16:53:37 CDT 2007


I can't think of a way to test that the fields added after Win95/NT4 are 
not used when the cbSize is invalid. That's especially hard as Wine 
currently doesn't use them. So I'm only testing that an invalid cbSize 
does work. If I'll think of a better test I'll send a patch.
-------------- next part --------------
From a9520b6d02d1583c8804980dcaad3c82d4d93ca0 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Miko=C5=82aj_Zalewski?= <mikolaj at zalewski.pl>
Date: Wed, 9 May 2007 23:47:36 +0200
Subject: [PATCH] shell32: handle invalid cbSize in Shell_NotifyIcon[AW] (3rd try)

---
 dlls/shell32/systray.c         |   37 ++++++++++++--
 dlls/shell32/tests/Makefile.in |    5 +-
 dlls/shell32/tests/systray.c   |  106 ++++++++++++++++++++++++++++++++++++++++
 programs/explorer/systray.c    |    2 +-
 4 files changed, 143 insertions(+), 7 deletions(-)

diff --git a/dlls/shell32/systray.c b/dlls/shell32/systray.c
index 824eeb9..869aaef 100644
--- a/dlls/shell32/systray.c
+++ b/dlls/shell32/systray.c
@@ -45,6 +45,20 @@ static const WCHAR classname[] = /* Shell_TrayWnd */ {'S','h','e','l','l','_','T
 BOOL WINAPI Shell_NotifyIconA(DWORD dwMessage, PNOTIFYICONDATAA pnid)
 {
     NOTIFYICONDATAW nidW;
+    INT cbSize;
+    
+    /* Validate the cbSize as Windows XP does */
+    if (pnid->cbSize != NOTIFYICONDATAA_V1_SIZE &&
+        pnid->cbSize != NOTIFYICONDATAA_V2_SIZE &&
+        pnid->cbSize != NOTIFYICONDATAA_V3_SIZE &&
+        pnid->cbSize != sizeof(NOTIFYICONDATAA))
+    {
+        WARN("Invalid cbSize (%d) - using only Win95 fields (size=%d)\n",
+            pnid->cbSize, NOTIFYICONDATAA_V1_SIZE);
+        cbSize = NOTIFYICONDATAA_V1_SIZE;
+    }
+    else
+        cbSize = pnid->cbSize;
 
     ZeroMemory(&nidW, sizeof(nidW));
     nidW.cbSize = sizeof(nidW);
@@ -58,7 +72,7 @@ BOOL WINAPI Shell_NotifyIconA(DWORD dwMessage, PNOTIFYICONDATAA pnid)
     if (pnid->uFlags & NIF_TIP)
         MultiByteToWideChar(CP_ACP, 0, pnid->szTip, -1, nidW.szTip, sizeof(nidW.szTip)/sizeof(WCHAR));
 
-    if (pnid->cbSize >= NOTIFYICONDATAA_V2_SIZE)
+    if (cbSize >= NOTIFYICONDATAA_V2_SIZE)
     {
         nidW.dwState      = pnid->dwState;
         nidW.dwStateMask  = pnid->dwStateMask;
@@ -74,10 +88,10 @@ BOOL WINAPI Shell_NotifyIconA(DWORD dwMessage, PNOTIFYICONDATAA pnid)
         nidW.dwInfoFlags = pnid->dwInfoFlags;
     }
     
-    if (pnid->cbSize >= NOTIFYICONDATAA_V3_SIZE)
+    if (cbSize >= NOTIFYICONDATAA_V3_SIZE)
         nidW.guidItem = pnid->guidItem;
 
-    if (pnid->cbSize >= sizeof(NOTIFYICONDATAA))
+    if (cbSize >= sizeof(NOTIFYICONDATAA))
         nidW.hBalloonIcon = pnid->hBalloonIcon;
     return Shell_NotifyIconW(dwMessage, &nidW);
 }
@@ -92,6 +106,21 @@ BOOL WINAPI Shell_NotifyIconW(DWORD dwMessage, PNOTIFYICONDATAW nid)
     char *buffer = NULL;
 
     TRACE("dwMessage = %d, nid->cbSize=%d\n", dwMessage, nid->cbSize);
+    
+    /* Validate the cbSize so that WM_COPYDATA doesn't crash the application */
+    if (nid->cbSize != NOTIFYICONDATAW_V1_SIZE &&
+        nid->cbSize != NOTIFYICONDATAW_V2_SIZE &&
+        nid->cbSize != NOTIFYICONDATAW_V3_SIZE &&
+        nid->cbSize != sizeof(NOTIFYICONDATAW))
+    {
+        NOTIFYICONDATAW newNid;
+
+        WARN("Invalid cbSize (%d) - using only Win95 fields (size=%d)\n",
+            nid->cbSize, NOTIFYICONDATAW_V1_SIZE);
+        CopyMemory(&newNid, nid, NOTIFYICONDATAW_V1_SIZE);
+        newNid.cbSize = NOTIFYICONDATAW_V1_SIZE;
+        return Shell_NotifyIconW(dwMessage, &newNid);
+    }
 
     tray = FindWindowExW(0, NULL, classname, NULL);
     if (!tray) return FALSE;
@@ -131,7 +160,7 @@ BOOL WINAPI Shell_NotifyIconW(DWORD dwMessage, PNOTIFYICONDATAW nid)
         }
         cds.lpData = buffer;
 
-        memcpy(buffer, nid, sizeof(*nid));
+        memcpy(buffer, nid, nid->cbSize);
         buffer += nid->cbSize;
         memcpy(buffer, &bmMask, sizeof(bmMask));
         buffer += sizeof(bmMask);
diff --git a/dlls/shell32/tests/Makefile.in b/dlls/shell32/tests/Makefile.in
index 94b0c85..3acd432 100644
--- a/dlls/shell32/tests/Makefile.in
+++ b/dlls/shell32/tests/Makefile.in
@@ -3,7 +3,7 @@ TOPOBJDIR = ../../..
 SRCDIR    = @srcdir@
 VPATH     = @srcdir@
 TESTDLL   = shell32.dll
-IMPORTS   = shell32 ole32 oleaut32 shlwapi advapi32 kernel32
+IMPORTS   = shell32 ole32 oleaut32 shlwapi advapi32 user32 gdi32 kernel32
 EXTRALIBS = -luuid
 
 CTESTS = \
@@ -13,7 +13,8 @@ CTESTS = \
 	shlexec.c \
 	shlfileop.c \
 	shlfolder.c \
-	string.c
+	string.c \
+	systray.c
 
 @MAKE_TEST_RULES@
 
diff --git a/dlls/shell32/tests/systray.c b/dlls/shell32/tests/systray.c
new file mode 100644
index 0000000..ff6f56d
--- /dev/null
+++ b/dlls/shell32/tests/systray.c
@@ -0,0 +1,106 @@
+/* Unit tests for systray
+ *
+ * Copyright 2007 Mikolaj Zalewski
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+#define _WIN32_IE 0x600
+#include <assert.h>
+#include <stdarg.h>
+
+#include <windows.h>
+
+#include "wine/test.h"
+
+
+static HWND hMainWnd;
+
+void test_cbsize()
+{
+    NOTIFYICONDATAW nidW;
+    NOTIFYICONDATAA nidA;
+
+    ZeroMemory(&nidW, sizeof(nidW));
+    nidW.cbSize = NOTIFYICONDATAW_V1_SIZE;
+    nidW.hWnd = hMainWnd;
+    nidW.uID = 1;
+    nidW.uFlags = NIF_ICON|NIF_MESSAGE;
+    nidW.hIcon = LoadIcon(NULL, IDI_APPLICATION);
+    nidW.uCallbackMessage = WM_USER+17;
+    ok(Shell_NotifyIconW(NIM_ADD, &nidW), "NIM_ADD failed!\n");
+
+    /* using an invalid cbSize does work */
+    nidW.cbSize = 3;
+    nidW.hWnd = hMainWnd;
+    nidW.uID = 1;
+    ok(Shell_NotifyIconW(NIM_DELETE, &nidW), "NIM_DELETE failed!\n");
+    /* as icon doesn't exist anymore - now there will be an error */
+    nidW.cbSize = sizeof(nidW);
+    /* wine currently doesn't return error code put prints an ERR(...) */
+    todo_wine ok(!Shell_NotifyIconW(NIM_DELETE, &nidW), "The icon was not deleted\n");
+
+    /* same for Shell_NotifyIconA */
+    ZeroMemory(&nidA, sizeof(nidA));
+    nidA.cbSize = NOTIFYICONDATAA_V1_SIZE;
+    nidA.hWnd = hMainWnd;
+    nidA.uID = 1;
+    nidA.uFlags = NIF_ICON|NIF_MESSAGE;
+    nidA.hIcon = LoadIcon(NULL, IDI_APPLICATION);
+    nidA.uCallbackMessage = WM_USER+17;
+    ok(Shell_NotifyIconA(NIM_ADD, &nidA), "NIM_ADD failed!\n");
+
+    /* using an invalid cbSize does work */
+    nidA.cbSize = 3;
+    nidA.hWnd = hMainWnd;
+    nidA.uID = 1;
+    ok(Shell_NotifyIconA(NIM_DELETE, &nidA), "NIM_DELETE failed!\n");
+    /* as icon doesn't exist anymore - now there will be an error */
+    nidA.cbSize = sizeof(nidA);
+    /* wine currently doesn't return error code put prints an ERR(...) */
+    todo_wine ok(!Shell_NotifyIconA(NIM_DELETE, &nidA), "The icon was not deleted\n");
+}
+
+START_TEST(systray)
+{
+    WNDCLASSA wc;
+    MSG msg;
+    RECT rc;
+  
+    wc.style = CS_HREDRAW | CS_VREDRAW;
+    wc.cbClsExtra = 0;
+    wc.cbWndExtra = 0;
+    wc.hInstance = GetModuleHandleA(NULL);
+    wc.hIcon = NULL;
+    wc.hCursor = LoadCursorA(NULL, MAKEINTRESOURCEA(IDC_IBEAM));
+    wc.hbrBackground = GetSysColorBrush(COLOR_WINDOW);
+    wc.lpszMenuName = NULL;
+    wc.lpszClassName = "MyTestWnd";
+    wc.lpfnWndProc = DefWindowProc;
+    RegisterClassA(&wc);
+    
+    hMainWnd = CreateWindowExA(0, "MyTestWnd", "Blah", WS_OVERLAPPEDWINDOW, 
+      CW_USEDEFAULT, CW_USEDEFAULT, 680, 260, NULL, NULL, GetModuleHandleA(NULL), 0);
+    GetClientRect(hMainWnd, &rc);
+    ShowWindow(hMainWnd, SW_SHOW);
+
+    test_cbsize();
+
+    PostQuitMessage(0);
+    while(GetMessageA(&msg,0,0,0)) {
+        TranslateMessage(&msg);
+        DispatchMessageA(&msg);
+    }
+    DestroyWindow(hMainWnd);
+}
diff --git a/programs/explorer/systray.c b/programs/explorer/systray.c
index b93d209..1814bce 100644
--- a/programs/explorer/systray.c
+++ b/programs/explorer/systray.c
@@ -290,7 +290,7 @@ static void delete_icon(const NOTIFYICONDATAW *nid)
    
     if (!icon)
     {
-        WINE_ERR("invalid tray icon ID specified: %ud\n", nid->uID);
+        WINE_ERR("invalid tray icon ID specified: %u\n", nid->uID);
         return;
     }
 
-- 
1.4.4.2


More information about the wine-patches mailing list