[PATCH v2 0/2] MR10993: crypt32: handle CERT_NCRYPT_KEY_HANDLE_PROP_ID setting as implemented (not documented)
The [documentation for CertSetCertificateContextProperty](https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-cer...) says pvData should contain a pointer to an NCRYPT_KEY_HANDLE, however the actual implementation of this function expects just the handle. This is easily shown by passing a "dead beef" value in pvData, it will not be dereferenced on Windows and the value retrieved later will be the same as provided. Yes, this means there are consumers of this function out there passing a stack pointer which is dutifully stored away. It's only when that value is used that anything goes wrong. -- v2: crypt32: make CertContext_SetProperty of CERT_NCRYPT_KEY_HANDLE_PROP_ID behave as implemented, not as documented. Add tests to demonstrate undocumented behavior of crypt32 CERT_NCRYPT_KEY_HANDLE_PROP_ID setting https://gitlab.winehq.org/wine/wine/-/merge_requests/10993
From: Trent Waddington <trent.waddington@tensorworks.com.au> --- dlls/crypt32/tests/cert.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dlls/crypt32/tests/cert.c b/dlls/crypt32/tests/cert.c index 57222591836..4771be721b7 100644 --- a/dlls/crypt32/tests/cert.c +++ b/dlls/crypt32/tests/cert.c @@ -549,7 +549,7 @@ static void testCertProperties(void) HCRYPTPROV_OR_NCRYPT_KEY_HANDLE retrievedHandle = 0; ret = CertSetCertificateContextProperty(context, - CERT_NCRYPT_KEY_HANDLE_PROP_ID, 0, &ncryptHandle); + CERT_NCRYPT_KEY_HANDLE_PROP_ID, 0, (void *)ncryptHandle); ok(ret, "CertSetCertificateContextProperty failed: %08lx\n", GetLastError()); /* Verify the key context was set with CERT_NCRYPT_KEY_SPEC */ @@ -558,8 +558,8 @@ static void testCertProperties(void) ret = CertGetCertificateContextProperty(context, CERT_KEY_CONTEXT_PROP_ID, &keyContext, &size); ok(ret, "CertGetCertificateContextProperty failed: %08lx\n", GetLastError()); - ok(keyContext.hNCryptKey != 0, - "Expected non-zero hNCryptKey, got 0\n"); + ok(keyContext.hNCryptKey == ncryptHandle, + "Expected key context to now be hNCryptKey, got something else\n"); ok(keyContext.dwKeySpec == CERT_NCRYPT_KEY_SPEC, "Expected dwKeySpec CERT_NCRYPT_KEY_SPEC, got %lx\n", keyContext.dwKeySpec); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10993
From: Trent Waddington <trent.waddington@tensorworks.com.au> --- dlls/crypt32/cert.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/crypt32/cert.c b/dlls/crypt32/cert.c index 5014e2a5600..c37432822a2 100644 --- a/dlls/crypt32/cert.c +++ b/dlls/crypt32/cert.c @@ -880,7 +880,7 @@ static BOOL CertContext_SetProperty(cert_t *cert, DWORD dwPropId, break; } keyContext.cbSize = sizeof(keyContext); - keyContext.hNCryptKey = *(const NCRYPT_KEY_HANDLE *)pvData; + keyContext.hNCryptKey = (NCRYPT_KEY_HANDLE)pvData; keyContext.dwKeySpec = CERT_NCRYPT_KEY_SPEC; ret = CertContext_SetKeyContextProperty(cert->base.properties, &keyContext); break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10993
This merge request was approved by Hans Leidekker. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10993
participants (3)
-
Hans Leidekker (＠hans) -
Trent Waddington
-
Trent Waddington (＠trent.waddington)