Convert SID to String

Question

I'm not a C++ developer, so I guess it's just my fault that my program doesn't work. I want to look up the SID for a windows group and return a readable SID.

wchar_t* SpcLookupName(LPCTSTR lpszSystemName, LPCTSTR lpszAccountName) {

PSID         Sid;
DWORD        cbReferencedDomainName, cbSid;
LPTSTR       ReferencedDomainName;
SID_NAME_USE eUse;

cbReferencedDomainName = cbSid = 0;
if (LookupAccountName(lpszSystemName, lpszAccountName, 0, &cbSid,
                    0, &cbReferencedDomainName, &eUse)) {
    SetLastError(ERROR_NONE_MAPPED);
    return 0;
}

if (GetLastError(  ) != ERROR_INSUFFICIENT_BUFFER) return 0;

if (!(Sid = (PSID)LocalAlloc(LMEM_FIXED, cbSid))) return 0;

ReferencedDomainName = (LPTSTR)LocalAlloc(LMEM_FIXED, cbReferencedDomainName);

if (!ReferencedDomainName) {
    LocalFree(Sid);
    return 0;
}

if (!LookupAccountName(lpszSystemName, lpszAccountName, Sid, &cbSid,
                     ReferencedDomainName, &cbReferencedDomainName, &eUse)) {
    LocalFree(ReferencedDomainName);
    LocalFree(Sid);
    return 0;
}

wchar_t* psz;

// Loading ConvertSidToStringSid
typedef BOOL (WINAPI *tConvertSidToStringSid)(PSID,wchar_t*);

tConvertSidToStringSid pConvertSidToStringSid=0;

HINSTANCE handle = ::LoadLibrary("Advapi32.dll");

pConvertSidToStringSid = (tConvertSidToStringSid) ::GetProcAddress(handle, "ConvertSidToStringSidA");

if(pConvertSidToStringSid(Sid, psz)){                               
    return psz;
}
}

My problem is that the function returns only some strange characters and not the SID, why?

Answer 1

There are a couple of obvious things wrong with your code...

1) the prototype for ConvertSidToStringSid() is BOOL ConvertSidToStringSid(PSID Sid, LPTSTR *StringSid); which means that your typedef should be typedef BOOL (WINAPI *tConvertSidToStringSid)(PSID,LPTSTR *);

2) you are looking up the ANSI version and passing a wchar_t *... You should either look up the wide character version ConvertSidToStringSid() or, a better solution, IMHO, look up ConvertSidToStringSid and use a TCHAR * rather than a wchar_t * as this will work no matter what your unicode compile settings are, and it matches the typedef.

3) finally due to the broken typedef you're being allowed to pass the wrong data type to the function. It wants a LPTSTR * and you're passing it a LPTSTR (well, actually you're passing a wchar_t *... The LPTSTR is in fact the unicode aware type that maps to a wchar_t * if you're using wide characters but the function that you are calling wants a pointer to that pointer and not the pointer itself.

So, the fixed code is:

typedef BOOL (WINAPI *tConvertSidToStringSid)(PSID,LPTSTR*);

tConvertSidToStringSid pConvertSidToStringSid=0;

HINSTANCE handle = ::LoadLibrary("Advapi32.dll");



pConvertSidToStringSid = (tConvertSidToStringSid) ::GetProcAddress(handle, #ConvertSidToStringSid);

if(pConvertSidToStringSid(Sid, &psz)){                               
    return psz;
}

Note the change to the typedef, the change to the GetProcAddress call (where we use the # to stringize the function name (which will either be ConvertSidToStringSidA or ConvertSidToStringSidW depending on your unicode settings) and the change to pass the address of psz rather than the pointer itself.

And now your only problem is that you're potentially leaking that psz unless you are making sure you free it in the caller with a call to LocalFree().

Note that depending on the calling code, you might want to convert the typedef into wchar_t ** rather than LPTSTR * and then use the ConvertSidToStringSidW version to force the result to always be a wide character string.

Answer 2

Maybe your pConvertSidToStringSid(Sid, psz) call is not successful and SpcLookupName() return value is undefined because you don't return anything.

And why are you mixing ConvertSidToStringSidA with wchar_t? It would be better to use ConvertSidToStringSid from Sddl.h directly:

#define _WIN32_WINNT 0x0500
#include <Sddl.h>

You should also return LPTSTR from your function instead of wchar_t *

Answer 3

You use ConvertSidToStringSidA which will return ANSI string, at the same time you are expecting wchar_t*. Try to use ConvertSidToStringSidW instead. Or just include Sddl.h and use ConvertSidToStringSid.

Answer 4

You are calling the ANSI version of the function when you want the Unicode version "ConvertSidToStringSidW".

Also you have a small memory leak as you aren't alway freeing Sid.