[Fwd: [advapi32] Correctly handle incorrect buffer sizes in LookupAccountNameA (RESEND)]

[Zac Brown]

Hi,

Could I get some feedback on this patch as to why it wasn't accepted? Just
trying to see where I've gone wrong and what I can do to improve the patch.

Any feedback would be greatly appreciated. Thanks!

-Zac

-------- Original Message --------
Subject: [advapi32] Correctly handle incorrect buffer sizes in
LookupAccountNameA (RESEND)
Date: Fri, 21 Dec 2007 02:32:45 -0600
From: Zac Brown <[EMAIL PROTECTED]>
Reply-To: wine-devel@winehq.org
To: [EMAIL PROTECTED]

Hi,

Changelog:
* Correctly handle incorrect buffer sizes in LookupAccountNameA

Notes:
* Fixes bug 10612

-Zac Brown



---
 dlls/advapi32/security.c       |   20 ++++++++++++++------
 dlls/advapi32/tests/security.c |    4 ++--
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/dlls/advapi32/security.c b/dlls/advapi32/security.c
index 89498c2..1bdd630 100644
--- a/dlls/advapi32/security.c
+++ b/dlls/advapi32/security.c
@@ -2443,6 +2443,9 @@ LookupAccountNameA(
     UNICODE_STRING lpSystemW;
     UNICODE_STRING lpAccountW;
     LPWSTR lpReferencedDomainNameW = NULL;
+    DWORD domsize;
+    
+    domsize = *cbReferencedDomainName;
 
     RtlCreateUnicodeStringFromAsciiz(&lpSystemW, system);
     RtlCreateUnicodeStringFromAsciiz(&lpAccountW, account);
@@ -2451,13 +2454,16 @@ LookupAccountNameA(
         lpReferencedDomainNameW = HeapAlloc(GetProcessHeap(), 0, *cbReferencedDomainName * sizeof(WCHAR));
 
     ret = LookupAccountNameW(lpSystemW.Buffer, lpAccountW.Buffer, sid, cbSid, lpReferencedDomainNameW,
-        cbReferencedDomainName, name_use);
+        &domsize, name_use);
 
-    if (ret && lpReferencedDomainNameW)
+    if (ret && lpReferencedDomainNameW && domsize < *cbReferencedDomainName)
     {
-        WideCharToMultiByte(CP_ACP, 0, lpReferencedDomainNameW, *cbReferencedDomainName,
+        WideCharToMultiByte(CP_ACP, 0, lpReferencedDomainNameW, -1,
             ReferencedDomainName, *cbReferencedDomainName, NULL, NULL);
+        
     }
+    
+    *cbReferencedDomainName = domsize;
 
     RtlFreeUnicodeString(&lpSystemW);
     RtlFreeUnicodeString(&lpAccountW);
@@ -2505,18 +2511,20 @@ BOOL WINAPI LookupAccountNameW( LPCWSTR lpSystemName, LPCWSTR lpAccountName, PSI
        SetLastError(ERROR_INSUFFICIENT_BUFFER);
        ret = FALSE;
     }
-    *cbSid = GetLengthSid(pSid);
     
     if (ReferencedDomainName != NULL && (*cchReferencedDomainName > strlenW(dm)))
       strcpyW(ReferencedDomainName, dm);
 
-    if (*cchReferencedDomainName <= strlenW(dm))
+    if (*cchReferencedDomainName <= strlenW(dm) || *cbSid < GetLengthSid(pSid))
     {
        SetLastError(ERROR_INSUFFICIENT_BUFFER);
+       *cchReferencedDomainName = strlenW(dm)+1;
        ret = FALSE;
     }
+    else
+       *cchReferencedDomainName = strlenW(dm);
 
-    *cchReferencedDomainName = strlenW(dm)+1;
+    *cbSid = GetLengthSid(pSid);
 
     FreeSid(pSid);
 
diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c
index e9f4795..214bfa8 100644
--- a/dlls/advapi32/tests/security.c
+++ b/dlls/advapi32/tests/security.c
@@ -1536,10 +1536,10 @@ static void test_LookupAccountName(void)
     {
         ok(!lstrcmp(account, user_name), "Expected %s, got %s\n", user_name, account);
         ok(!lstrcmp(domain, sid_dom), "Expected %s, got %s\n", sid_dom, domain);
-        ok(domain_size == domain_save - 1, "Expected %d, got %d\n", domain_save - 1, domain_size);
-        ok(lstrlen(domain) == domain_size, "Expected %d\n", lstrlen(domain));
         ok(sid_use == SidTypeUser, "Expected SidTypeUser, got %d\n", sid_use);
     }
+    ok(domain_size == domain_save - 1, "Expected %d, got %d\n", domain_save - 1, domain_size);
+    ok(lstrlen(domain) == domain_size, "Expected %d\n", lstrlen(domain));
     domain_size = domain_save;
 
     /* NULL Sid with zero sid size */
-- 
1.5.2.5