[PATCH 0/3] MR358: atl: Avoid NULL pointer reference in AtlComModuleGetClassObject().
Fix crash in CAXA CAD when clicking component library. Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/358
From: Zhiyi Zhang <zzhang(a)codeweavers.com> Fix crash in CAXA CAD when clicking component library. Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> --- configure.ac | 1 + dlls/atl/atl.c | 4 +- dlls/atl100/tests/atl.c | 27 +++++++++++ dlls/atl110/tests/Makefile.in | 6 +++ dlls/atl110/tests/atl.c | 86 +++++++++++++++++++++++++++++++++++ 5 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 dlls/atl110/tests/Makefile.in create mode 100644 dlls/atl110/tests/atl.c diff --git a/configure.ac b/configure.ac index 9ad1b19164c..f26e9cde350 100644 --- a/configure.ac +++ b/configure.ac @@ -2394,6 +2394,7 @@ WINE_CONFIG_MAKEFILE(dlls/atl/tests) WINE_CONFIG_MAKEFILE(dlls/atl100) WINE_CONFIG_MAKEFILE(dlls/atl100/tests) WINE_CONFIG_MAKEFILE(dlls/atl110) +WINE_CONFIG_MAKEFILE(dlls/atl110/tests) WINE_CONFIG_MAKEFILE(dlls/atl80) WINE_CONFIG_MAKEFILE(dlls/atl80/tests) WINE_CONFIG_MAKEFILE(dlls/atl90) diff --git a/dlls/atl/atl.c b/dlls/atl/atl.c index 87a24e33ed7..d501e7a6d76 100644 --- a/dlls/atl/atl.c +++ b/dlls/atl/atl.c @@ -482,7 +482,7 @@ HRESULT WINAPI AtlComModuleGetClassObject(_ATL_COM_MODULE *pm, REFCLSID rclsid, return E_INVALIDARG; for(iter = pm->m_ppAutoObjMapFirst; iter < pm->m_ppAutoObjMapLast; iter++) { - if(IsEqualCLSID((*iter)->pclsid, rclsid) && (*iter)->pfnGetClassObject) { + if(*iter && IsEqualCLSID((*iter)->pclsid, rclsid) && (*iter)->pfnGetClassObject) { if(!(*iter)->pCF) hres = (*iter)->pfnGetClassObject((*iter)->pfnCreateInstance, &IID_IUnknown, (void**)&(*iter)->pCF); if((*iter)->pCF) @@ -507,7 +507,7 @@ HRESULT WINAPI AtlComModuleGetClassObject(_ATL_COM_MODULE *pm, REFCLSID rclsid, return E_INVALIDARG; for(iter = pm->m_ppAutoObjMapFirst; iter < pm->m_ppAutoObjMapLast; iter++) { - if(IsEqualCLSID((*iter)->pclsid, rclsid) && (*iter)->pfnGetClassObject) { + if(*iter && IsEqualCLSID((*iter)->pclsid, rclsid) && (*iter)->pfnGetClassObject) { if(!(*iter)->pCache->pCF) hres = (*iter)->pfnGetClassObject((*iter)->pfnCreateInstance, &IID_IUnknown, (void**)&(*iter)->pCache->pCF); if((*iter)->pCache->pCF) diff --git a/dlls/atl100/tests/atl.c b/dlls/atl100/tests/atl.c index e161878f8ea..e002af0d24c 100644 --- a/dlls/atl100/tests/atl.c +++ b/dlls/atl100/tests/atl.c @@ -1062,6 +1062,32 @@ static void test_AtlAxCreateControl(void) DestroyWindow(hwnd); } +static void test_AtlComModuleGetClassObject(void) +{ + _ATL_OBJMAP_ENTRY *null_entry = NULL; + _ATL_COM_MODULE module; + HRESULT hr; + void *ret; + + /* Test NULL module */ + hr = AtlComModuleGetClassObject(NULL, &GUID_NULL, &IID_NULL, &ret); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + + /* Test NULL m_ppAutoObjMapFirst and m_ppAutoObjMapLast */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = NULL; + module.m_ppAutoObjMapLast = NULL; + hr = AtlComModuleGetClassObject(&module, &GUID_NULL, &IID_NULL, &ret); + ok(hr == CLASS_E_CLASSNOTAVAILABLE, "Unexpected hr %#lx.\n", hr); + + /* Test m_ppAutoObjMapFirst and m_ppAutoObjMapLast both pointing to a NULL entry */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = &null_entry; + module.m_ppAutoObjMapLast = &null_entry; + hr = AtlComModuleGetClassObject(&module, &GUID_NULL, &IID_NULL, &ret); + ok(hr == CLASS_E_CLASSNOTAVAILABLE, "Unexpected hr %#lx.\n", hr); +} + START_TEST(atl) { if (!register_class()) @@ -1077,6 +1103,7 @@ START_TEST(atl) test_ax_win(); test_AtlAxAttachControl(); test_AtlAxCreateControl(); + test_AtlComModuleGetClassObject(); CoUninitialize(); } diff --git a/dlls/atl110/tests/Makefile.in b/dlls/atl110/tests/Makefile.in new file mode 100644 index 00000000000..528c9180dc8 --- /dev/null +++ b/dlls/atl110/tests/Makefile.in @@ -0,0 +1,6 @@ +TESTDLL = atl110.dll +IMPORTS = uuid ole32 +EXTRADEFS = -D_ATL_VER=_ATL_VER_110 + +C_SRCS = \ + atl.c diff --git a/dlls/atl110/tests/atl.c b/dlls/atl110/tests/atl.c new file mode 100644 index 00000000000..12f3518b429 --- /dev/null +++ b/dlls/atl110/tests/atl.c @@ -0,0 +1,86 @@ +/* + * Copyright 2022 Zhiyi Zhang for CodeWeavers + * + * 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 + */ + +#include <stdarg.h> +#include <stdio.h> + +#define COBJMACROS +#define CONST_VTABLE + +#include <windef.h> +#include <winbase.h> +#include <winuser.h> +#include <atlbase.h> + +#include <wine/test.h> + +static HRESULT (WINAPI *pAtlComModuleGetClassObject)(_ATL_COM_MODULE *, REFCLSID, REFIID, void **); + +static HMODULE atl110; + +static void init_functions(void) +{ + atl110 = LoadLibraryA("atl110.dll"); + +#define X(f) p##f = (void *)GetProcAddress(atl110, #f); + X(AtlComModuleGetClassObject) +#undef X +} + +static void test_AtlComModuleGetClassObject(void) +{ + _ATL_OBJMAP_ENTRY_EX *null_entry = NULL; + _ATL_COM_MODULE module; + HRESULT hr; + void *ret; + + if (!pAtlComModuleGetClassObject) + { + win_skip("AtlComModuleGetClassObject() is unavailable.\n"); + return; + } + + /* Test NULL module */ + hr = pAtlComModuleGetClassObject(NULL, &GUID_NULL, &IID_NULL, &ret); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + + /* Test NULL m_ppAutoObjMapFirst and m_ppAutoObjMapLast */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = NULL; + module.m_ppAutoObjMapLast = NULL; + hr = pAtlComModuleGetClassObject(&module, &GUID_NULL, &IID_NULL, &ret); + ok(hr == CLASS_E_CLASSNOTAVAILABLE, "Unexpected hr %#lx.\n", hr); + + /* Test m_ppAutoObjMapFirst and m_ppAutoObjMapLast both pointing to a NULL entry */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = &null_entry; + module.m_ppAutoObjMapLast = &null_entry; + hr = pAtlComModuleGetClassObject(&module, &GUID_NULL, &IID_NULL, &ret); + ok(hr == CLASS_E_CLASSNOTAVAILABLE, "Unexpected hr %#lx.\n", hr); +} + +START_TEST(atl) +{ + CoInitialize(NULL); + init_functions(); + + test_AtlComModuleGetClassObject(); + + FreeLibrary(atl110); + CoUninitialize(); +} -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/358
From: Zhiyi Zhang <zzhang(a)codeweavers.com> Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/atl/atl.c | 4 ++-- dlls/atl100/tests/atl.c | 28 ++++++++++++++++++++++++++++ dlls/atl110/tests/atl.c | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/dlls/atl/atl.c b/dlls/atl/atl.c index d501e7a6d76..5a302621d60 100644 --- a/dlls/atl/atl.c +++ b/dlls/atl/atl.c @@ -538,7 +538,7 @@ HRESULT WINAPI AtlComModuleRegisterClassObjects(_ATL_COM_MODULE *module, DWORD c return E_INVALIDARG; for(iter = module->m_ppAutoObjMapFirst; iter < module->m_ppAutoObjMapLast; iter++) { - if(!(*iter)->pfnGetClassObject) + if(!(*iter) || !(*iter)->pfnGetClassObject) continue; hres = (*iter)->pfnGetClassObject((*iter)->pfnCreateInstance, &IID_IUnknown, (void**)&unk); @@ -566,7 +566,7 @@ HRESULT WINAPI AtlComModuleRegisterClassObjects(_ATL_COM_MODULE *module, DWORD c return E_INVALIDARG; for(iter = module->m_ppAutoObjMapFirst; iter < module->m_ppAutoObjMapLast; iter++) { - if(!(*iter)->pfnGetClassObject) + if(!(*iter) || !(*iter)->pfnGetClassObject) continue; hres = (*iter)->pfnGetClassObject((*iter)->pfnCreateInstance, &IID_IUnknown, (void**)&unk); diff --git a/dlls/atl100/tests/atl.c b/dlls/atl100/tests/atl.c index e002af0d24c..7da6e5a9cd6 100644 --- a/dlls/atl100/tests/atl.c +++ b/dlls/atl100/tests/atl.c @@ -1088,6 +1088,33 @@ static void test_AtlComModuleGetClassObject(void) ok(hr == CLASS_E_CLASSNOTAVAILABLE, "Unexpected hr %#lx.\n", hr); } +static void test_AtlComModuleRegisterClassObjects(void) +{ + _ATL_OBJMAP_ENTRY *null_entry = NULL; + _ATL_COM_MODULE module; + HRESULT hr; + + /* Test NULL module */ + hr = AtlComModuleRegisterClassObjects(NULL, CLSCTX_INPROC_SERVER, REGCLS_MULTIPLEUSE); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + + /* Test NULL m_ppAutoObjMapFirst and m_ppAutoObjMapLast */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = NULL; + module.m_ppAutoObjMapLast = NULL; + hr = AtlComModuleRegisterClassObjects(&module, CLSCTX_INPROC_SERVER, REGCLS_MULTIPLEUSE); + todo_wine_if(hr == S_OK) + ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr); + + /* Test m_ppAutoObjMapFirst and m_ppAutoObjMapLast both pointing to a NULL entry */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = &null_entry; + module.m_ppAutoObjMapLast = &null_entry; + hr = AtlComModuleRegisterClassObjects(&module, CLSCTX_INPROC_SERVER, REGCLS_MULTIPLEUSE); + todo_wine_if(hr == S_OK) + ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr); +} + START_TEST(atl) { if (!register_class()) @@ -1104,6 +1131,7 @@ START_TEST(atl) test_AtlAxAttachControl(); test_AtlAxCreateControl(); test_AtlComModuleGetClassObject(); + test_AtlComModuleRegisterClassObjects(); CoUninitialize(); } diff --git a/dlls/atl110/tests/atl.c b/dlls/atl110/tests/atl.c index 12f3518b429..57e7f5b9b21 100644 --- a/dlls/atl110/tests/atl.c +++ b/dlls/atl110/tests/atl.c @@ -30,6 +30,7 @@ #include <wine/test.h> static HRESULT (WINAPI *pAtlComModuleGetClassObject)(_ATL_COM_MODULE *, REFCLSID, REFIID, void **); +static HRESULT (WINAPI *pAtlComModuleRegisterClassObjects)(_ATL_COM_MODULE *, DWORD, DWORD); static HMODULE atl110; @@ -39,6 +40,7 @@ static void init_functions(void) #define X(f) p##f = (void *)GetProcAddress(atl110, #f); X(AtlComModuleGetClassObject) + X(AtlComModuleRegisterClassObjects) #undef X } @@ -74,12 +76,46 @@ static void test_AtlComModuleGetClassObject(void) ok(hr == CLASS_E_CLASSNOTAVAILABLE, "Unexpected hr %#lx.\n", hr); } +static void test_AtlComModuleRegisterClassObjects(void) +{ + _ATL_OBJMAP_ENTRY_EX *null_entry = NULL; + _ATL_COM_MODULE module; + HRESULT hr; + + if (!pAtlComModuleRegisterClassObjects) + { + win_skip("AtlComModuleRegisterClassObjects() is unavailable.\n"); + return; + } + + /* Test NULL module */ + hr = pAtlComModuleRegisterClassObjects(NULL, CLSCTX_INPROC_SERVER, REGCLS_MULTIPLEUSE); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + + /* Test NULL m_ppAutoObjMapFirst and m_ppAutoObjMapLast */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = NULL; + module.m_ppAutoObjMapLast = NULL; + hr = pAtlComModuleRegisterClassObjects(&module, CLSCTX_INPROC_SERVER, REGCLS_MULTIPLEUSE); + todo_wine_if(hr == S_OK) + ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr); + + /* Test m_ppAutoObjMapFirst and m_ppAutoObjMapLast both pointing to a NULL entry */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = &null_entry; + module.m_ppAutoObjMapLast = &null_entry; + hr = pAtlComModuleRegisterClassObjects(&module, CLSCTX_INPROC_SERVER, REGCLS_MULTIPLEUSE); + todo_wine_if(hr == S_OK) + ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr); +} + START_TEST(atl) { CoInitialize(NULL); init_functions(); test_AtlComModuleGetClassObject(); + test_AtlComModuleRegisterClassObjects(); FreeLibrary(atl110); CoUninitialize(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/358
From: Zhiyi Zhang <zzhang(a)codeweavers.com> Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/atl/atl.c | 6 ++++++ dlls/atl100/tests/atl.c | 26 ++++++++++++++++++++++++++ dlls/atl110/tests/atl.c | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/dlls/atl/atl.c b/dlls/atl/atl.c index 5a302621d60..32e0c722f8b 100644 --- a/dlls/atl/atl.c +++ b/dlls/atl/atl.c @@ -598,6 +598,9 @@ HRESULT WINAPI AtlComModuleRevokeClassObjects(_ATL_COM_MODULE *module) return E_INVALIDARG; for(iter = module->m_ppAutoObjMapFirst; iter < module->m_ppAutoObjMapLast; iter++) { + if(!(*iter)) + continue; + hres = CoRevokeClassObject((*iter)->dwRegister); if(FAILED(hres)) return hres; @@ -617,6 +620,9 @@ HRESULT WINAPI AtlComModuleRevokeClassObjects(_ATL_COM_MODULE *module) return E_INVALIDARG; for(iter = module->m_ppAutoObjMapFirst; iter < module->m_ppAutoObjMapLast; iter++) { + if(!(*iter)) + continue; + hres = CoRevokeClassObject((*iter)->pCache->dwRegister); if(FAILED(hres)) return hres; diff --git a/dlls/atl100/tests/atl.c b/dlls/atl100/tests/atl.c index 7da6e5a9cd6..a40c38df25a 100644 --- a/dlls/atl100/tests/atl.c +++ b/dlls/atl100/tests/atl.c @@ -1115,6 +1115,31 @@ static void test_AtlComModuleRegisterClassObjects(void) ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr); } +static void test_AtlComModuleRevokeClassObjects(void) +{ + _ATL_OBJMAP_ENTRY *null_entry = NULL; + _ATL_COM_MODULE module; + HRESULT hr; + + /* Test NULL module */ + hr = AtlComModuleRevokeClassObjects(NULL); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + + /* Test NULL m_ppAutoObjMapFirst and m_ppAutoObjMapLast */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = NULL; + module.m_ppAutoObjMapLast = NULL; + hr = AtlComModuleRevokeClassObjects(&module); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + /* Test m_ppAutoObjMapFirst and m_ppAutoObjMapLast both pointing to a NULL entry */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = &null_entry; + module.m_ppAutoObjMapLast = &null_entry; + hr = AtlComModuleRevokeClassObjects(&module); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); +} + START_TEST(atl) { if (!register_class()) @@ -1132,6 +1157,7 @@ START_TEST(atl) test_AtlAxCreateControl(); test_AtlComModuleGetClassObject(); test_AtlComModuleRegisterClassObjects(); + test_AtlComModuleRevokeClassObjects(); CoUninitialize(); } diff --git a/dlls/atl110/tests/atl.c b/dlls/atl110/tests/atl.c index 57e7f5b9b21..79958f2bd7a 100644 --- a/dlls/atl110/tests/atl.c +++ b/dlls/atl110/tests/atl.c @@ -31,6 +31,7 @@ static HRESULT (WINAPI *pAtlComModuleGetClassObject)(_ATL_COM_MODULE *, REFCLSID, REFIID, void **); static HRESULT (WINAPI *pAtlComModuleRegisterClassObjects)(_ATL_COM_MODULE *, DWORD, DWORD); +static HRESULT (WINAPI *pAtlComModuleRevokeClassObjects)(_ATL_COM_MODULE *); static HMODULE atl110; @@ -41,6 +42,7 @@ static void init_functions(void) #define X(f) p##f = (void *)GetProcAddress(atl110, #f); X(AtlComModuleGetClassObject) X(AtlComModuleRegisterClassObjects) + X(AtlComModuleRevokeClassObjects) #undef X } @@ -109,6 +111,37 @@ static void test_AtlComModuleRegisterClassObjects(void) ok(hr == S_FALSE, "Unexpected hr %#lx.\n", hr); } +static void test_AtlComModuleRevokeClassObjects(void) +{ + _ATL_OBJMAP_ENTRY_EX *null_entry = NULL; + _ATL_COM_MODULE module; + HRESULT hr; + + if (!pAtlComModuleRevokeClassObjects) + { + win_skip("AtlComModuleRevokeClassObjects() is unavailable.\n"); + return; + } + + /* Test NULL module */ + hr = pAtlComModuleRevokeClassObjects(NULL); + ok(hr == E_INVALIDARG, "Unexpected hr %#lx.\n", hr); + + /* Test NULL m_ppAutoObjMapFirst and m_ppAutoObjMapLast */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = NULL; + module.m_ppAutoObjMapLast = NULL; + hr = pAtlComModuleRevokeClassObjects(&module); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + + /* Test m_ppAutoObjMapFirst and m_ppAutoObjMapLast both pointing to a NULL entry */ + module.cbSize = sizeof(module); + module.m_ppAutoObjMapFirst = &null_entry; + module.m_ppAutoObjMapLast = &null_entry; + hr = pAtlComModuleRevokeClassObjects(&module); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); +} + START_TEST(atl) { CoInitialize(NULL); @@ -116,6 +149,7 @@ START_TEST(atl) test_AtlComModuleGetClassObject(); test_AtlComModuleRegisterClassObjects(); + test_AtlComModuleRevokeClassObjects(); FreeLibrary(atl110); CoUninitialize(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/358
Jacek Caban (@jacek) commented about dlls/atl110/tests/atl.c:
+#define CONST_VTABLE + +#include <windef.h> +#include <winbase.h> +#include <winuser.h> +#include <atlbase.h> + +#include <wine/test.h> + +static HRESULT (WINAPI *pAtlComModuleGetClassObject)(_ATL_COM_MODULE *, REFCLSID, REFIID, void **); + +static HMODULE atl110; + +static void init_functions(void) +{ + atl110 = LoadLibraryA("atl110.dll"); Could we use import lib instead of dynamic loading?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/358#note_3131
On Fri Jul 1 18:24:51 2022 +0000, Jacek Caban wrote:
Could we use import lib instead of dynamic loading? MinGW doesn't find the import lib for atl110.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/358#note_3153
On Sat Jul 2 01:14:43 2022 +0000, Zhiyi Zhang wrote:
MinGW doesn't find the import lib for atl110. Sure, but we use Wine importlibs for building tests and we may just add it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/358#note_3446
participants (3)
-
Jacek Caban (＠jacek) -
Zhiyi Zhang
-
Zhiyi Zhang (＠zhiyi)