I spent few days chasing a crash that manifests as structured exception 0xC0000374
(heap corruption)... Of course, reproducible only in customer environment.
Narrowed it down to this (very simplified) code:
DWORD cchName = 0, cchDomain = 0;
SID_NAME_USE type;
if (!LookupAccountSidW(NULL, pSid, NULL, &cchName, NULL, &cchDomain, &type))
{
if (GetLastError() != ERROR_INSUFFICIENT_BUFFER)
<bail via exception or return>;
}
cout << cchName << ":" << cchDomain << " -> ";
DWORD cchName1 = (cchName + 1), cchDomain1 = (cchDomain + 1);
LPWSTR pName = ... allocate cchName1 WCHARs ...;
LPWSTR pDomain = ... allocate cchDomain1 WCHARs ...;
if (!LookupAccountSidW(NULL, pSid, pName, &cchName1, pDomain, &cchDomain1, &type))
<bail via exception or return>;
cout << cchName1 << ":" << cchDomain1 << endl;
... deallocate pDomain; // <- here Application Verifier detects corrupted block
... deallocate pName;
Please, ignore possibility of memory leak (code is simplified). Also, note that according to MSDN over-allocating by 1 symbol is not necessary. But let me describe what I see in debugger...
For all (but one) SIDs encountered it prints out things like 16:7 -> 15:6
or 6:7 -> 5:6
and everything is dandy. Basically, first call returns required buffer sizes (including terminating NUL
), second one returns number of symbols written (excluding NUL
) into provided buffers (which are over-allocated by 1, btw).
Now, one specific SID results in 6:3 -> 5:2
output. But when I look at pDomain
buffer (which is 4 symbols long) I see truncated domain name ABCD
(actual domain name is 6 symbols) followed by NUL
(which corrupts heap control structures). So LookupAccountSidW
claims it has written only 2(+1) symbols, while in reality is has written 4(+1) symbols into a buffer that is 4 symbols long.
From my standpoint it is a clear bug in LookupAccountSidW
, but I would really like to figure out how that SID differ from others. Maybe it was migrated from another (shorter) domain?
P.S. It is Windows 10 (10.0.14393.2969)
P.P.S. SID is S-1-5-21-<3-part domain id>-<user id>
I came across this issue, so I'm documenting my findings for whoever has the misfortune to fall into this trap.
It appears that there is a case where LookupAccountSidA will cause a heap overrun in certain environments.
It is possible to migrate users from one domain to another. The SIDs can remain as part of the domain for historical reasons. LookupAccountSidA is supposed to support looking up these SIDs, and it does so, except that it will overrun the heap internally, when the name of the old domain is longer than the new one.
Under the hood, LookupAccountSidA calls LsaLookupSids2 which gets a PLSA_TRANSLATED_NAME (the names array) and a PLSA_REFERENCED_DOMAIN_LIST. The issue here is that LookupAccountSidA makes use of the wrong array position in order to set up the size required for the domain data. If memory serves, it uses position zero where it should be something other than that. So, if the domain in position zero has a name shorter than the one associated with the SID you're looking up, LookupAccountSidA will overwrite the heap, all on its own.
The only work-around is to pass two buffers of 256 chars, and fill-initialize them to '\0'. Pass these in for your name and domain arrays, and do not trust the size that is returned for the domain. Copy these out of the buffers you get back, and careful not to overrun your buffers on read.
The better option, of course, is to use LsaLookupSids2 and do Unicode-to-multibyte conversion if necessary.
PS: A representative from Microsoft told me that this was a bug that was introduced between Windows 7 and 8. I don't know whether or not this affects LookupAccountSidW, but it most certainly is possible.