shell32: handle invalid cbSize in Shell_NotifyIcon[AW] (fixed, with
testcase)
Mikołaj Zalewski
mikolaj at zalewski.pl
Tue May 8 15:54:28 CDT 2007
As I've understood from wine-devel to catch exceptions in tests I need
to write my own handler. My handler is more compilcated than in
ntdll/tests/exception.c as I can't assume the next instruction after the
invalid one will be a 'ret' but is simplier than wine __TRY/__EXCEPT as
I don't try to filter exception but catch all of them (I hope this is
not a problem with stack guard pages or something like that). I'm also
not using siglongjump but longjmp to be able to compile it with msvcrt.
I haven't written an exception handler before but I hope it's correct.
The exception code is in macros not to obfuscate the test code.
As tests on Windows XP have shown, if the size is incorrect it uses
the Win95/NT4 structure size. Later I've tested it on Windows Vista and
it handles it differently - if there are additional flags in uFlags, it
uses a larger structure size. But as the Windows XP behaviour seems more
robust to me and most programs are written to be compatible with XP, so
I've stayed with the XP behaviour.
-------------- next part --------------
From 708e3b6c1b7923b65450528e2d6e48d3444720d3 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Miko=C5=82aj_Zalewski?= <mikolaj at zalewski.pl>
Date: Tue, 8 May 2007 22:38:02 +0200
Subject: [PATCH] shell32: handle invalid cbSize in Shell_NotifyIcon[AW] (fixed, with testcase)
---
dlls/shell32/systray.c | 37 ++++++-
dlls/shell32/tests/Makefile.in | 5 +-
dlls/shell32/tests/systray.c | 214 ++++++++++++++++++++++++++++++++++++++++
programs/explorer/systray.c | 2 +-
4 files changed, 251 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..58a0702
--- /dev/null
+++ b/dlls/shell32/tests/systray.c
@@ -0,0 +1,214 @@
+/* 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 <setjmp.h>
+
+#include "wine/test.h"
+
+/* Exception handling - written to support both gcc and Windows */
+static struct _TEB * (WINAPI *pNtCurrentTeb)(void);
+static void (*pRtlUnwind)(PVOID TargetFrame, PVOID TargetIp, PEXCEPTION_RECORD ExceptionRecord, PVOID ReturnValue);
+
+/* _EXCEPTION_REGISTRATION_RECORD and PEXCEPTION_HANDLER are defined in Wine headers but not in Platform SDK so we're
+ * defining out own types with an additional underscore - copied from Wine winnt.h */
+struct __EXCEPTION_REGISTRATION_RECORD;
+
+typedef DWORD (*_PEXCEPTION_HANDLER)(PEXCEPTION_RECORD,struct __EXCEPTION_REGISTRATION_RECORD*,
+ PCONTEXT,struct __EXCEPTION_REGISTRATION_RECORD **);
+
+struct __EXCEPTION_REGISTRATION_RECORD
+{
+ struct __EXCEPTION_REGISTRATION_RECORD *Prev;
+ _PEXCEPTION_HANDLER Handler;
+};
+
+typedef struct tagTEST_EXC_FRAME
+{
+ struct __EXCEPTION_REGISTRATION_RECORD frame;
+ BOOL handler_called;
+ DWORD code;
+ jmp_buf jmp;
+} TEST_EXC_FRAME;
+
+DWORD test_exception_handler( EXCEPTION_RECORD *record, struct __EXCEPTION_REGISTRATION_RECORD *frame,
+ CONTEXT *context, struct __EXCEPTION_REGISTRATION_RECORD **pdispatcher )
+{
+ NT_TIB *tib = (NT_TIB *)pNtCurrentTeb();
+ TEST_EXC_FRAME *test_frame = (TEST_EXC_FRAME *)frame;
+
+ test_frame->handler_called = TRUE;
+ test_frame->code = record->ExceptionCode;
+ pRtlUnwind( frame, 0, record, 0 );
+ tib->ExceptionList = (void *)frame->Prev;
+ longjmp( test_frame->jmp, 1 );
+}
+
+#define BEGIN_CATCH_EXCEPTIONS \
+ { \
+ TEST_EXC_FRAME test_frame; \
+ NT_TIB *tib = (NT_TIB *)pNtCurrentTeb(); \
+ test_frame.handler_called = FALSE; \
+ do { \
+ test_frame.frame.Handler = test_exception_handler; \
+ test_frame.frame.Prev = (struct __EXCEPTION_REGISTRATION_RECORD *)tib->ExceptionList; \
+ if (setjmp(test_frame.jmp)) /* exception handler called longjmp */ \
+ break; \
+ tib->ExceptionList = (struct _EXCEPTION_REGISTRATION_RECORD *)&test_frame.frame; \
+
+#define ON_EXCEPTION \
+ } while (0); \
+ tib->ExceptionList = (void *)test_frame.frame.Prev; \
+ if (test_frame.handler_called) \
+ { \
+ DWORD exceptionCode = test_frame.code;
+
+#define END_CATCH_EXCEPTIONS \
+ } \
+ }
+
+int init_exceptions()
+{
+ pNtCurrentTeb = (void *)GetProcAddress( GetModuleHandleA("ntdll.dll"), "NtCurrentTeb" );
+ pRtlUnwind = (void *)GetProcAddress( GetModuleHandleA("ntdll.dll"), "RtlUnwind" );
+ return (pNtCurrentTeb != NULL) && (pRtlUnwind != NULL);
+}
+
+static HWND hMainWnd;
+
+#define expect_eq(val, exp, type, format) { type ret = (val); ok(ret == (exp), #val " value " format " expected " format "\n", ret, (exp)); }
+
+void test_cbsize()
+{
+ int i = -1000;
+ SYSTEM_INFO info;
+ void *mem;
+ NOTIFYICONDATAW *pnid;
+ NOTIFYICONDATAA *pnidA;
+ BOOL unicode = TRUE;
+ DWORD dw;
+
+ if (!init_exceptions())
+ {
+ skip("No exceptions - can't do cbSize tests\n");
+ return;
+ }
+
+ GetSystemInfo(&info);
+ ok((mem = VirtualAlloc(NULL, 2*info.dwPageSize, MEM_COMMIT, PAGE_READWRITE)) != NULL, "VirtualAlloc failed\n");
+ ok(VirtualProtect((char *)mem + info.dwPageSize, info.dwPageSize, PAGE_NOACCESS, &dw), "VirtualProtect failed\n");
+ pnid = (PNOTIFYICONDATAW)(((char *)mem) + (info.dwPageSize - NOTIFYICONDATAW_V1_SIZE));
+
+ ZeroMemory(pnid, NOTIFYICONDATAW_V1_SIZE);
+ pnid->cbSize = NOTIFYICONDATAW_V1_SIZE;
+ pnid->hWnd = hMainWnd;
+ pnid->uID = 1;
+ pnid->uFlags = NIF_ICON|NIF_MESSAGE;
+ pnid->hIcon = LoadIcon(NULL, IDI_APPLICATION);
+ pnid->uCallbackMessage = WM_USER+17;
+
+ ok(Shell_NotifyIconW(NIM_ADD, pnid), "NIM_ADD failed!\n");
+
+ /* when using an invalid cbSize, the Windows XP Shell_NotifyIcon will use only the fields present in Windows 95 */
+ BEGIN_CATCH_EXCEPTIONS
+ {
+ unicode = TRUE;
+ for (i = -100; i < 1536; i+=4)
+ {
+ pnid->cbSize = i;
+ pnid->uFlags = NIF_TIP|NIF_INFO;
+ pnid->szTip[0] = 'a' + (i>0 ? i%26 : 0);
+ pnid->szTip[1] = 0;
+ if (i != 936 && i != 952 && i != 956)
+ ok(Shell_NotifyIconW(NIM_MODIFY, pnid), "Shell_NotifyIconW failed for i=%d\n", i);
+ }
+
+ pnid = NULL;
+ unicode = FALSE;
+ pnidA = (PNOTIFYICONDATAA)(((char *)mem) + (info.dwPageSize - NOTIFYICONDATAA_V1_SIZE));
+ ZeroMemory(pnidA, NOTIFYICONDATAA_V1_SIZE);
+ pnidA->cbSize = NOTIFYICONDATAA_V1_SIZE;
+ pnidA->hWnd = hMainWnd;
+ pnidA->uID = 1;
+ for (i = -100; i < 1000; i+=4)
+ {
+ pnidA->cbSize = i;
+ pnidA->uFlags = NIF_TIP|NIF_INFO;
+ pnidA->szTip[0] = 'a' + (i>0 ? i%26 : 0);
+ pnidA->szTip[1] = 0;
+ if (i != 488 && i != 504 && i != 508)
+ ok(Shell_NotifyIconA(NIM_MODIFY, pnidA), "Shell_NotifyIconA failed for i=%d\n", i);
+ }
+ }
+ ON_EXCEPTION
+ {
+ ok(FALSE, "Exception occured - Shell_NotifyIcon used too much data for i=%d, unicode=%d? (code=%x)\n",
+ i, unicode, exceptionCode);
+ }
+ END_CATCH_EXCEPTIONS
+
+ /* using an invalid cbSize does work */
+ pnidA = mem;
+ pnidA->cbSize = 3;
+ pnidA->hWnd = hMainWnd;
+ pnidA->uID = 1;
+ ok(Shell_NotifyIconA(NIM_DELETE, pnidA), "NIM_DELETE failed!\n");
+ /* as icon doesn't exist anymore - now there will be an error */
+ pnidA->cbSize = NOTIFYICONDATAA_V1_SIZE;
+ /* wine currently doesn't return error code put prints an ERR(...) */
+ todo_wine ok(!Shell_NotifyIconA(NIM_DELETE, pnidA), "The icon was not deleted\n");
+
+ VirtualFree(mem, 2*info.dwPageSize, MEM_DECOMMIT);
+}
+
+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