[PATCH 0/1] MR10395: crypt32: Don't access CERT_CHAIN_ENGINE_CONFIG::dwExclusiveFlags without checking size.

[Yuxuan Shui (＠yshui)]

Found by ASan.

Example in test `crypt32:chain`, `testCreateCertChainEngine`. Pointer to a `CERT_CHAIN_ENGINE_CONFIG_NO_EXCLUSIVE_ROOT` is passed to `CertCreateCertificateChainEngine`. Accessing `dwExclusiveFlags` unconditionally is out-of-bound.

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

[PATCH 1/1] crypt32: Don't access CERT_CHAIN_ENGINE_CONFIG::dwExclusiveFlags without checking size.

[Yuxuan Shui]

From: Yuxuan Shui <[e-mail]>

Found by ASan.
---
 dlls/crypt32/chain.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/dlls/crypt32/chain.c b/dlls/crypt32/chain.c
index 6e0e55b9e5a..0694abaf15d 100644
--- a/dlls/crypt32/chain.c
+++ b/dlls/crypt32/chain.c
@@ -239,7 +239,6 @@ BOOL WINAPI CertCreateCertificateChainEngine(PCERT_CHAIN_ENGINE_CONFIG pConfig,
     TRACE("hExclusiveRoot %p\n", pConfig->hExclusiveRoot);
     TRACE("hExclusiveTrustedPeople %p\n", pConfig->hExclusiveTrustedPeople);
     TRACE("dwExclusiveFlags %lx\n", pConfig->dwExclusiveFlags);
-    if (pConfig->dwExclusiveFlags) FIXME("dwExclusiveFlags %lx not supported\n", pConfig->dwExclusiveFlags);
 
     if (pConfig->cbSize != sizeof(CERT_CHAIN_ENGINE_CONFIG_NO_EXCLUSIVE_ROOT)
      && pConfig->cbSize != sizeof(CERT_CHAIN_ENGINE_CONFIG))
@@ -247,6 +246,10 @@ BOOL WINAPI CertCreateCertificateChainEngine(PCERT_CHAIN_ENGINE_CONFIG pConfig,
         SetLastError(E_INVALIDARG);
         return FALSE;
     }
+
+    if (pConfig->cbSize == sizeof(CERT_CHAIN_ENGINE_CONFIG) && pConfig->dwExclusiveFlags)
+        FIXME("dwExclusiveFlags %lx not supported\n", pConfig->dwExclusiveFlags);
+
     ret = CRYPT_CheckRestrictedRoot(pConfig->hRestrictedRoot);
     if (!ret)
     {
-- 
GitLab

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

[Yuxuan Shui (＠yshui)]

Should I move the `TRACE`s after the size check too?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10395#note_132971

[Hans Leidekker (＠hans)]

Yes, the last 3 traces should be moved too.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10395#note_132978