advapi32: Destination length -1 means no limit.

Alex Henrie alexhenrie24 at gmail.com
Wed Jan 30 23:31:16 CST 2013


I ran all the tests with and without my kernel32 patch and found only
one test it broke, in advapi32. The test passed 0xdeadbeef as a
destination buffer length, which overflowed the signed int and became
-559038737. It turns out that in this case, a negative destination
buffer length causes Windows to assume that the buffer is infinitely
long. Wine was, because of the WideCharToMultiByte bug, correctly
handling length < -1, but was not correctly handling length == -1. This
patch fixes the length == -1 bug and in all cases avoids passing
WideCharToMultiByte a negative destination length.

I changed the original test to use -1000 instead of -559038737. Feel
free to change it back if you think the old value was better.

Once this patch is applied, the kernel32 patch can be applied without
any test regressions. If after that users report regressions, I'd be
happy to fix them.

-Alex

---
 dlls/advapi32/crypt.c       |   32 +++++++++++++++++++-------------
 dlls/advapi32/tests/crypt.c |   11 ++++++++++-
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/dlls/advapi32/crypt.c b/dlls/advapi32/crypt.c
index 5f44728..3eb4213 100644
--- a/dlls/advapi32/crypt.c
+++ b/dlls/advapi32/crypt.c
@@ -27,6 +27,7 @@
 #include "config.h"
 #include "wine/port.h"
 
+#include <limits.h>
 #include <time.h>
 #include <stdlib.h>
 #include <stdio.h>
@@ -109,33 +110,38 @@ static inline PWSTR CRYPT_GetTypeKeyName(DWORD dwType, BOOL user)
 	return keyname;
 }
 
-/* CRYPT_UnicodeTOANSI
+/* CRYPT_UnicodeToANSI
  * wstr - unicode string
- * str - pointer to ANSI string
- * strsize - size of buffer pointed to by str or -1 if we have to do the allocation
+ * str - pointer to ANSI string or pointer to null pointer if we have to do the allocation
+ * strsize - size of buffer pointed to by str
  *
  * returns TRUE if unsuccessful, FALSE otherwise.
  * if wstr is NULL, returns TRUE and sets str to NULL! Value of str should be checked after call
  */
 static inline BOOL CRYPT_UnicodeToANSI(LPCWSTR wstr, LPSTR* str, int strsize)
 {
-	int count;
-
 	if (!wstr)
 	{
 		*str = NULL;
 		return TRUE;
 	}
-	count = WideCharToMultiByte(CP_ACP, 0, wstr, -1, NULL, 0, NULL, NULL);
-	if (strsize == -1)
-		*str = CRYPT_Alloc(count * sizeof(CHAR));
-	else
-		count = min( count, strsize );
+
+	if (!*str)
+	{
+		strsize = WideCharToMultiByte(CP_ACP, 0, wstr, -1, NULL, 0, NULL, NULL);
+		*str = CRYPT_Alloc(strsize * sizeof(CHAR));
+	}
+	else if (strsize < 0)
+	{
+		strsize = INT_MAX; /* windows will pretend that the buffer is infinitely long */
+	}
+
 	if (*str)
 	{
-		WideCharToMultiByte(CP_ACP, 0, wstr, -1, *str, count, NULL, NULL);
+		WideCharToMultiByte(CP_ACP, 0, wstr, -1, *str, strsize, NULL, NULL);
 		return TRUE;
 	}
+
 	SetLastError(ERROR_NOT_ENOUGH_MEMORY);
 	return FALSE;
 }
@@ -481,13 +487,13 @@ BOOL WINAPI CryptAcquireContextW (HCRYPTPROV *phProv, LPCWSTR pszContainer,
 		goto error;
 	}
 	pProv->pVTable->dwProvType = dwProvType;
-	if(!CRYPT_UnicodeToANSI(provname, &provnameA, -1))
+	if(!CRYPT_UnicodeToANSI(provname, &provnameA, 0))
 	{
 		/* CRYPT_UnicodeToANSI calls SetLastError */
 		goto error;
 	}
 	pProv->pVTable->pszProvName = provnameA;
-	if(!CRYPT_UnicodeToANSI(pszContainer, &pszContainerA, -1))
+	if(!CRYPT_UnicodeToANSI(pszContainer, &pszContainerA, 0))
 	{
 		/* CRYPT_UnicodeToANSI calls SetLastError */
 		goto error;
diff --git a/dlls/advapi32/tests/crypt.c b/dlls/advapi32/tests/crypt.c
index 4820d27..5f4d4e2 100644
--- a/dlls/advapi32/tests/crypt.c
+++ b/dlls/advapi32/tests/crypt.c
@@ -561,7 +561,16 @@ static void test_enum_providers(void)
 	if (!(provider = LocalAlloc(LMEM_ZEROINIT, providerLen)))
 		return;
 		
-	providerLen = 0xdeadbeef;
+	providerLen = -1;
+	result = pCryptEnumProvidersA(dwIndex, NULL, 0, &type, provider, &providerLen);
+	ok(result, "expected TRUE, got %d\n", result);
+	ok(type==dwType, "expected %d, got %d\n", dwType, type);
+	if (pszProvName)
+	    ok(!strcmp(pszProvName, provider), "expected %s, got %s\n", pszProvName, provider);
+	ok(cbName==providerLen, "expected %d, got %d\n", cbName, providerLen);
+
+	providerLen = -1000;
+	provider[0] = 0;
 	result = pCryptEnumProvidersA(dwIndex, NULL, 0, &type, provider, &providerLen);
 	ok(result, "expected TRUE, got %d\n", result);
 	ok(type==dwType, "expected %d, got %d\n", dwType, type);
-- 
1.7.10.4



More information about the wine-patches mailing list