[PATCH 0/1] MR10917: oleaut32/olepropframe.c: Initialize page_info in OleCreatePropertyFrameIndirect

[Ivan Ivlev (＠iviv)]

Uninitialized page_info can cause corruption in nearby stack memory: IPropertyPage_GetPageInfo appears to depend on page_info.cb being valid.  I got NULL returned from CreatePropertySheetPageW, because property_sheet_page.dwSize was corrupted (set to 0). 

This commit fixes an error by initializing page_info with 0 and setting page_info.cb = sizeof(PROPPAGEINFO)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10917

[PATCH 1/1] oleaut32/olepropframe.c: Initialize page_info in OleCreatePropertyFrameIndirect

[Ivan Ivlev]

From: Ivan Ivlev <[e-mail]>

Signed-off-by: Ivan Ivlev <[e-mail]>
---
 dlls/oleaut32/olepropframe.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/dlls/oleaut32/olepropframe.c b/dlls/oleaut32/olepropframe.c
index cad1a2ad5c2..f6c32ced623 100644
--- a/dlls/oleaut32/olepropframe.c
+++ b/dlls/oleaut32/olepropframe.c
@@ -263,6 +263,9 @@ HRESULT WINAPI OleCreatePropertyFrameIndirect(LPOCPFIPARAMS lpParams)
     for(i=0; i<lpParams->cPages; i++) {
         PROPPAGEINFO page_info;
 
+        memset(&page_info, 0, sizeof(PROPPAGEINFO));
+        page_info.cb = sizeof(PROPPAGEINFO);
+
         res = CoCreateInstance(&lpParams->lpPages[i], NULL, CLSCTX_INPROC_SERVER,
                 &IID_IPropertyPage, (void**)&property_page[i]);
         if(FAILED(res))
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/10917

[Nikolay Sivov (＠nsivov)]

Could you produce a standalone test program that shows what's being passed in there? No need to integrate it to wine tests, and for page classes it's the easiest to use temporary registration with CoRegisterClassObject(). 

We don't have any corresponding IPropertyPage implementation to quickly test with.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10917#note_140172

[Ivan Ivlev (＠iviv)]

[oleaut32_pageinfo_probe.c](/uploads/7dec8c7be0f7a82bf48becefbf415c37/oleaut32_pageinfo_probe.c)

Here is test program that outputs cb value.

Output for wine before fix:

GetPageInfo: cb=0 expected=48 OleCreatePropertyFrameIndirect returned 0

Output for windows/wine after fix:

GetPageInfo: cb=48 expected=48 OleCreatePropertyFrameIndirect returned 0

Built with: gcc oleaut32_pageinfo_probe.c -o oleaut32_pageinfo_probe.exe -loleaut32 -lole32 -luuid

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10917#note_141240

[Ivan Ivlev (＠iviv)]

[oleaut32_pageinfo_probe.exe](/uploads/1a08a354c5b7e8b217783285c998ef34/oleaut32_pageinfo_probe.exe)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10917#note_141241

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/oleaut32/olepropframe.c:
> for(i=0; i<lpParams->cPages; i++) {
>          PROPPAGEINFO page_info;
>
> +        memset(&page_info, 0, sizeof(PROPPAGEINFO));
> +        page_info.cb = sizeof(PROPPAGEINFO);
> +
>          res = CoCreateInstance(&lpParams->lpPages[i], NULL, CLSCTX_INPROC_SERVER,

Thank you for the test. Please move this right before GetPageInfo() and use sizeof(page_info) for the size value. Otherwise looks good.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10917#note_141255