[PATCH 0/1] MR1908: msi: pSignatureSettings uninitialized

[Philip Karlsson Gisslow (＠ripxorip)]

This field was not unitilized so it got a random value from
the stack causing a later crash when it was dereferenced in Wintrust
(SoftPubloadSignature).

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

[PATCH 1/1] msi: pSignatureSettings uninitialized

[Philip K. Gisslow]

From: "Philip K. Gisslow" <ripxorip(a)gmail.com>

This field was not unitilized so it got a random value from
the stack causing a later crash when it was dereferenced in Wintrust
(SoftPubloadSignature).
---
 dlls/msi/msi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dlls/msi/msi.c b/dlls/msi/msi.c
index db04babb899..886c5d3ba6f 100644
--- a/dlls/msi/msi.c
+++ b/dlls/msi/msi.c
@@ -2556,6 +2556,7 @@ HRESULT WINAPI MsiGetFileSignatureInformationW( const WCHAR *path, DWORD flags,
     data.pwszURLReference    = NULL;
     data.dwProvFlags         = 0;
     data.dwUIContext         = WTD_UICONTEXT_INSTALL;
+    data.pSignatureSettings  = NULL;
     hr = WinVerifyTrustEx( INVALID_HANDLE_VALUE, &generic_verify_v2, &data );
     *cert = NULL;
     if (FAILED(hr)) goto done;
-- 
GitLab

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

[Zebediah Figura (＠zfigura)]

Just to add some extra information for traceability (or for anyone else who had the same questions I did), this appears to be a regression from 9430954c8a7d. There are two other users in wine, but they both memset() the structure [which probably wouldn't be a bad idea here either, but, well, matter of taste.]

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1908#note_20341

[Nikolay Sivov (＠nsivov)]

memset-ing seems better so that we can remove some lines to initialize other members, and for consistency.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1908#note_20346

[Philip Karlsson Gisslow (＠ripxorip)]

On Thu Jan  5 06:28:33 2023 +0000, Zebediah Figura wrote:
> Just to add some extra information for traceability (or for anyone else
> who had the same questions I did), this appears to be a regression from
> 9430954c8a7d. There are two other users in wine, but they both memset()
> the structure [which probably wouldn't be a bad idea here either, but,
> well, matter of taste.]
> Thanks for your input. My initial approach was to initialize the struct i.e. cbData = {}. But since this is my first contribution I decided to keep the style as the code is already written (explicitly initialize the members).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/1908#note_20347