[PATCH 0/1] MR7218: crypt32/tests: Avoid use after free in testEmptyStore().

[Paul Gofman (＠gofman)]

Which is use after free since commit 278437b266ed97cebb8e1c14be6cfa2c4f440f07.

Thanks Bernhard Übelacker for spotting this (based on ASan validation).
For a general case I'd say we can have some tests intentionally doing use after free but it is not needed here.

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

[PATCH 1/1] crypt32/tests: Avoid use after free in testEmptyStore().

[Paul Gofman]

From: Paul Gofman <pgofman(a)codeweavers.com>

---
 dlls/crypt32/tests/store.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/dlls/crypt32/tests/store.c b/dlls/crypt32/tests/store.c
index fc52c2ce28d..3c0b6a166e1 100644
--- a/dlls/crypt32/tests/store.c
+++ b/dlls/crypt32/tests/store.c
@@ -2807,7 +2807,7 @@ static DWORD countCRLsInStore(HCERTSTORE store)
 
 static void testEmptyStore(void)
 {
-    const CERT_CONTEXT *cert, *cert2, *cert3;
+    const CERT_CONTEXT *cert, *cert2, *cert3, *copy;
     const CRL_CONTEXT *crl;
     const CTL_CONTEXT *ctl;
     HCERTSTORE store;
@@ -2837,9 +2837,12 @@ static void testEmptyStore(void)
 
     test_store_is_empty(cert->hCertStore);
 
-    res = CertDeleteCertificateFromStore(cert3);
+    copy = CertDuplicateCertificateContext(cert3);
+    ok(copy == cert3, "got %p, %p.\n", copy, cert3);
+    res = CertDeleteCertificateFromStore(copy);
     ok(res, "CertDeleteCertificateContextFromStore failed\n");
-    ok(cert3->hCertStore == cert->hCertStore, "Unexpected hCertStore\n");
+    ok(cert3->hCertStore == cert->hCertStore, "Unexpected hCertStore %p\n", cert3->hCertStore);
+    CertFreeCertificateContext(copy);
 
     store = CertOpenStore(CERT_STORE_PROV_MEMORY, 0, 0, CERT_STORE_CREATE_NEW_FLAG, NULL);
     ok(store != NULL, "CertOpenStore failed\n");
@@ -2849,9 +2852,12 @@ static void testEmptyStore(void)
     ok(cert3 && cert3 != cert2, "Unexpected cert3\n");
     ok(cert3->hCertStore == store, "Unexpected hCertStore\n");
 
-    res = CertDeleteCertificateFromStore(cert3);
+    copy = CertDuplicateCertificateContext(cert3);
+    ok(copy == cert3, "got %p, %p.\n", copy, cert3);
+    res = CertDeleteCertificateFromStore(copy);
     ok(res, "CertDeleteCertificateContextFromStore failed\n");
     ok(cert3->hCertStore == store, "Unexpected hCertStore\n");
+    CertFreeCertificateContext(copy);
 
     res = CertCloseStore(store, CERT_CLOSE_STORE_CHECK_FLAG);
     ok(res, "got error %#lx.\n", GetLastError());
-- 
GitLab

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

[Paul Gofman (＠gofman)]

This also can be tested with WINEDEBUG=warn+heap, that results in test failure without the patch.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7218#note_93079

[PATCH 0/1] MR7218: crypt32/tests: Avoid use after free in testEmptyStore(). - approved

[Hans Leidekker (＠hans)]

This merge request was approved by Hans Leidekker.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7218