[PATCH 0/1] MR11545: wintrust: Check for NULL pSgnr in WTHelperGetProvCertFromChain().

[Paul Gofman (＠gofman)]

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

[PATCH 1/1] wintrust: Check for NULL pSgnr in WTHelperGetProvCertFromChain().

[Paul Gofman]

From: Paul Gofman <[e-mail]>

---
 dlls/wintrust/tests/crypt.c   | 17 +++++++++++++++++
 dlls/wintrust/wintrust_main.c |  3 +++
 2 files changed, 20 insertions(+)

diff --git a/dlls/wintrust/tests/crypt.c b/dlls/wintrust/tests/crypt.c
index 84675dae8b7..fd9b4e450fb 100644
--- a/dlls/wintrust/tests/crypt.c
+++ b/dlls/wintrust/tests/crypt.c
@@ -25,6 +25,7 @@
 #include "windows.h"
 #include "wincrypt.h"
 #include "mscat.h"
+#include "wintrust.h"
 
 #include "wine/test.h"
 
@@ -1448,6 +1449,21 @@ static void test_sip(void)
     DeleteFileW(nameW);
 }
 
+static void test_WTHelperGetProvCertFromChain(void)
+{
+    CRYPT_PROVIDER_CERT *cert;
+
+    SetLastError(0xdeadbeef);
+    cert = WTHelperGetProvCertFromChain(NULL, 0);
+    ok(!cert, "got %p.\n", cert);
+    ok(GetLastError() == 0xdeadbeef, "got %lu.\n", GetLastError());
+
+    SetLastError(0xdeadbeef);
+    cert = WTHelperGetProvCertFromChain(NULL, 1);
+    ok(!cert, "got %p.\n", cert);
+    ok(GetLastError() == 0xdeadbeef, "got %lu.\n", GetLastError());
+}
+
 START_TEST(crypt)
 {
     char** myARGV;
@@ -1483,4 +1499,5 @@ START_TEST(crypt)
     test_create_catalog_file();
     test_CryptCATAdminAddRemoveCatalog();
     test_sip();
+    test_WTHelperGetProvCertFromChain();
 }
diff --git a/dlls/wintrust/wintrust_main.c b/dlls/wintrust/wintrust_main.c
index 22921006d4a..bfaa79c7654 100644
--- a/dlls/wintrust/wintrust_main.c
+++ b/dlls/wintrust/wintrust_main.c
@@ -759,6 +759,9 @@ CRYPT_PROVIDER_CERT * WINAPI WTHelperGetProvCertFromChain(
 
     TRACE("(%p %ld)\n", pSgnr, idxCert);
 
+    if (!pSgnr)
+        return NULL;
+
     if (idxCert >= pSgnr->csCertChain || !pSgnr->pasCertChain)
         return NULL;
     cert = &pSgnr->pasCertChain[idxCert];
-- 
GitLab

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

[Elizabeth Figura (＠zfigura)]

I think the reason this hadn't gotten upstream before is that it's clear it was only a symptom of some earlier failure.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11545#note_147510

[Paul Gofman (＠gofman)]

Can be in principle, but an app calls it directly. I am not saying that it completely works after that, but that goes much further and has probably quite different issues after. I presume since it is correct behaviour it is better to just fix it than reverse engineering obfuscated code to uncover the full story under that.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11545#note_147527

[Paul Gofman (＠gofman)]

Especially given in this case the earlier failure (if there is one) is most likely missing signatures on system dlls.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11545#note_147528