From bd0e74253cfc0b69360dc85d7528f637b463aed5 Mon Sep 17 00:00:00 2001 From: Juan Lang Date: Thu, 16 Sep 2004 20:27:52 +0000 Subject: [PATCH] - correct ConvertStringSidToSidW and ConvertSidToStringSidW, with tests - add ConvertStringSidToSidA - add missing exports for ConvertStringSidToSidA/W --- dlls/advapi32/advapi32.spec | 2 + dlls/advapi32/security.c | 95 +++++++++++++++++++++++----- dlls/advapi32/tests/security.c | 112 +++++++++++++++++++++++++++------ 3 files changed, 172 insertions(+), 37 deletions(-) diff --git a/dlls/advapi32/advapi32.spec b/dlls/advapi32/advapi32.spec index 1de323c4d4c..5afb67e3dc4 100644 --- a/dlls/advapi32/advapi32.spec +++ b/dlls/advapi32/advapi32.spec @@ -41,6 +41,8 @@ @ stdcall ControlService(long long ptr) @ stdcall ConvertSidToStringSidA(ptr ptr) @ stdcall ConvertSidToStringSidW(ptr ptr) +@ stdcall ConvertStringSidToSidA(ptr ptr) +@ stdcall ConvertStringSidToSidW(ptr ptr) @ stub ConvertStringSecurityDescriptorToSecurityDescriptorA #(str long ptr ptr) ConvertStringSecurityDescriptorToSecurityDescriptorA @ stdcall ConvertStringSecurityDescriptorToSecurityDescriptorW(wstr long ptr ptr) @ stdcall CopySid(long ptr ptr) diff --git a/dlls/advapi32/security.c b/dlls/advapi32/security.c index 715d9c4dc46..3cd44c95ae3 100644 --- a/dlls/advapi32/security.c +++ b/dlls/advapi32/security.c @@ -2187,8 +2187,11 @@ BOOL WINAPI ConvertStringSidToSidW(LPCWSTR StringSid, PSID* Sid) BOOL bret = FALSE; DWORD cBytes; + TRACE("%s, %p\n", debugstr_w(StringSid), Sid); if (GetVersion() & 0x80000000) SetLastError(ERROR_CALL_NOT_IMPLEMENTED); + else if (!StringSid || !Sid) + SetLastError(ERROR_INVALID_PARAMETER); else if (ParseStringSidToSid(StringSid, NULL, &cBytes)) { PSID pSid = *Sid = (PSID) LocalAlloc(0, cBytes); @@ -2197,7 +2200,33 @@ BOOL WINAPI ConvertStringSidToSidW(LPCWSTR StringSid, PSID* Sid) if (!bret) LocalFree(*Sid); } + TRACE("returning %s\n", bret ? "TRUE" : "FALSE"); + return bret; +} +/****************************************************************************** + * ConvertStringSidToSidA [ADVAPI32.@] + */ +BOOL WINAPI ConvertStringSidToSidA(LPCSTR StringSid, PSID* Sid) +{ + BOOL bret = FALSE; + + TRACE("%s, %p\n", debugstr_a(StringSid), Sid); + if (GetVersion() & 0x80000000) + SetLastError(ERROR_CALL_NOT_IMPLEMENTED); + else if (!StringSid || !Sid) + SetLastError(ERROR_INVALID_PARAMETER); + else + { + UINT len = MultiByteToWideChar(CP_ACP, 0, StringSid, -1, NULL, 0); + LPWSTR wStringSid = (LPWSTR)HeapAlloc(GetProcessHeap(), 0, + len * sizeof(WCHAR)); + + MultiByteToWideChar(CP_ACP, 0, StringSid, -1, wStringSid, len); + bret = ConvertStringSidToSidW(wStringSid, Sid); + HeapFree(GetProcessHeap(), 0, wStringSid); + } + TRACE("returning %s\n", bret ? "TRUE" : "FALSE"); return bret; } @@ -2215,8 +2244,7 @@ BOOL WINAPI ConvertSidToStringSidW( PSID pSid, LPWSTR *pstr ) { DWORD sz, i; LPWSTR str; - WCHAR fmt[] = { - 'S','-','%','u','-','%','2','X','%','2','X','%','X','%','X','%','X','%','X',0 }; + WCHAR fmt[] = { 'S','-','%','u','-','%','d',0 }; WCHAR subauthfmt[] = { '-','%','u',0 }; SID* pisid=pSid; @@ -2227,16 +2255,20 @@ BOOL WINAPI ConvertSidToStringSidW( PSID pSid, LPWSTR *pstr ) if (pisid->Revision != SDDL_REVISION) return FALSE; + if (pisid->IdentifierAuthority.Value[0] || + pisid->IdentifierAuthority.Value[1]) + { + FIXME("not matching MS' bugs\n"); + return FALSE; + } sz = 14 + pisid->SubAuthorityCount * 11; str = LocalAlloc( 0, sz*sizeof(WCHAR) ); - sprintfW( str, fmt, pisid->Revision, - pisid->IdentifierAuthority.Value[2], - pisid->IdentifierAuthority.Value[3], - pisid->IdentifierAuthority.Value[0]&0x0f, - pisid->IdentifierAuthority.Value[4]&0x0f, - pisid->IdentifierAuthority.Value[1]&0x0f, - pisid->IdentifierAuthority.Value[5]&0x0f); + sprintfW( str, fmt, pisid->Revision, MAKELONG( + MAKEWORD( pisid->IdentifierAuthority.Value[5], + pisid->IdentifierAuthority.Value[4] ), + MAKEWORD( pisid->IdentifierAuthority.Value[3], + pisid->IdentifierAuthority.Value[2] ) ) ); for( i=0; iSubAuthorityCount; i++ ) sprintfW( str + strlenW(str), subauthfmt, pisid->SubAuthority[i] ); *pstr = str; @@ -2297,37 +2329,65 @@ static BOOL ParseStringSidToSid(LPCWSTR StringSid, PSID pSid, LPDWORD cBytes) BOOL bret = FALSE; SID* pisid=pSid; + TRACE("%s, %p, %p\n", debugstr_w(StringSid), pSid, cBytes); if (!StringSid) { SetLastError(ERROR_INVALID_PARAMETER); + TRACE("StringSid is NULL, returning FALSE\n"); return FALSE; } *cBytes = ComputeStringSidSize(StringSid); if (!pisid) /* Simply compute the size */ + { + TRACE("only size requested, returning TRUE\n"); return TRUE; + } if (*StringSid != 'S' || *StringSid != '-') /* S-R-I-S-S */ { - int i = 0; - int csubauth = ((*cBytes - sizeof(SID)) / sizeof(DWORD)) + 1; + DWORD i = 0, identAuth; + DWORD csubauth = ((*cBytes - sizeof(SID)) / sizeof(DWORD)) + 1; StringSid += 2; /* Advance to Revision */ pisid->Revision = atoiW(StringSid); if (pisid->Revision != SDDL_REVISION) - goto lend; /* ERROR_INVALID_SID */ + { + TRACE("Revision %d is unknown\n", pisid->Revision); + goto lend; /* ERROR_INVALID_SID */ + } + if (csubauth == 0) + { + TRACE("SubAuthorityCount is 0\n"); + goto lend; /* ERROR_INVALID_SID */ + } pisid->SubAuthorityCount = csubauth; + /* Advance to identifier authority */ while (*StringSid && *StringSid != '-') - StringSid++; /* Advance to identifier authority */ + StringSid++; + if (*StringSid == '-') + StringSid++; - pisid->IdentifierAuthority.Value[5] = atoiW(StringSid); + /* MS' implementation can't handle values greater than 2^32 - 1, so + * we don't either; assume most significant bytes are always 0 + */ + pisid->IdentifierAuthority.Value[0] = 0; + pisid->IdentifierAuthority.Value[1] = 0; + identAuth = atoiW(StringSid); + pisid->IdentifierAuthority.Value[5] = identAuth & 0xff; + pisid->IdentifierAuthority.Value[4] = (identAuth & 0xff00) >> 8; + pisid->IdentifierAuthority.Value[3] = (identAuth & 0xff0000) >> 16; + pisid->IdentifierAuthority.Value[2] = (identAuth & 0xff000000) >> 24; + + /* Advance to first sub authority */ + while (*StringSid && *StringSid != '-') + StringSid++; + if (*StringSid == '-') + StringSid++; - if (pisid->IdentifierAuthority.Value[5] > 5) - goto lend; /* ERROR_INVALID_SID */ - while (*StringSid) { while (*StringSid && *StringSid != '-') @@ -2359,6 +2419,7 @@ lend: if (!bret) SetLastError(ERROR_INVALID_SID); + TRACE("returning %s\n", bret ? "TRUE" : "FALSE"); return bret; } diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index a143fd51d04..7cf90779fb8 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -25,36 +25,108 @@ #include "winbase.h" #include "winerror.h" #include "aclapi.h" +#include "winnt.h" typedef BOOL (WINAPI *fnConvertSidToStringSidA)( PSID pSid, LPSTR *str ); -typedef BOOL (WINAPI *fnConvertSidToStringSidW)( PSID pSid, LPWSTR *str ); +typedef BOOL (WINAPI *fnConvertStringSidToSidA)( LPCSTR str, PSID pSid ); -fnConvertSidToStringSidW pConvertSidToStringSidW; fnConvertSidToStringSidA pConvertSidToStringSidA; +fnConvertStringSidToSidA pConvertStringSidToSidA; + +struct sidRef +{ + SID_IDENTIFIER_AUTHORITY auth; + const char *refStr; +}; void test_sid() { - PSID psid; - LPWSTR str = NULL; - BOOL r; - SID_IDENTIFIER_AUTHORITY auth = { {6,7,0x1a,0x15,0x0e,0x1f} }; - WCHAR refstr[] = { 'S','-','1','-','1','A','1','5','6','E','7','F','-', - '1','2','3','4','5','-','0','-','4','2','9','4','9','6','7','2','9','5',0 }; - + struct sidRef refs[] = { + { { {0x00,0x00,0x33,0x44,0x55,0x66} }, "S-1-860116326-1" }, + { { {0x00,0x00,0x01,0x02,0x03,0x04} }, "S-1-16909060-1" }, + { { {0x00,0x00,0x00,0x01,0x02,0x03} }, "S-1-66051-1" }, + { { {0x00,0x00,0x00,0x00,0x01,0x02} }, "S-1-258-1" }, + { { {0x00,0x00,0x00,0x00,0x00,0x02} }, "S-1-2-1" }, + { { {0x00,0x00,0x00,0x00,0x00,0x0c} }, "S-1-12-1" }, + }; + const char noSubAuthStr[] = "S-1-5"; HMODULE hmod = GetModuleHandle("advapi32.dll"); + unsigned int i; + PSID psid = NULL; + BOOL r; + LPSTR str = NULL; - pConvertSidToStringSidW = (fnConvertSidToStringSidW) - GetProcAddress( hmod, "ConvertSidToStringSidW" ); - if( !pConvertSidToStringSidW ) + pConvertSidToStringSidA = (fnConvertSidToStringSidA) + GetProcAddress( hmod, "ConvertSidToStringSidA" ); + if( !pConvertSidToStringSidA ) return; - - r = AllocateAndInitializeSid( &auth, 3, 12345, 0,-1,0,0,0,0,0,&psid); - ok( r, "failed to allocate sid\n" ); - r = pConvertSidToStringSidW( psid, &str ); - ok( r, "failed to convert sid\n" ); - ok( !lstrcmpW( str, refstr ), "incorrect sid\n" ); - LocalFree( str ); - FreeSid( psid ); + pConvertStringSidToSidA = (fnConvertStringSidToSidA) + GetProcAddress( hmod, "ConvertStringSidToSidA" ); + if( !pConvertStringSidToSidA ) + return; + + r = pConvertStringSidToSidA( NULL, NULL ); + ok( !r, "expected failure with NULL parameters\n" ); + if( GetLastError() == ERROR_CALL_NOT_IMPLEMENTED ) + return; + ok( GetLastError() == ERROR_INVALID_PARAMETER, + "expected GetLastError() is ERROR_INVALID_PARAMETER, got %ld\n", + GetLastError() ); + + r = pConvertStringSidToSidA( refs[0].refStr, NULL ); + ok( !r && GetLastError() == ERROR_INVALID_PARAMETER, + "expected GetLastError() is ERROR_INVALID_PARAMETER, got %ld\n", + GetLastError() ); + + r = pConvertStringSidToSidA( NULL, &str ); + ok( !r && GetLastError() == ERROR_INVALID_PARAMETER, + "expected GetLastError() is ERROR_INVALID_PARAMETER, got %ld\n", + GetLastError() ); + + r = pConvertStringSidToSidA( noSubAuthStr, &psid ); + ok( !r, + "expected failure with no sub authorities\n" ); + ok( GetLastError() == ERROR_INVALID_SID, + "expected GetLastError() is ERROR_INVALID_SID, got %ld\n", + GetLastError() ); + + for( i = 0; i < sizeof(refs) / sizeof(refs[0]); i++ ) + { + PISID pisid; + + r = AllocateAndInitializeSid( &refs[i].auth, 1,1,0,0,0,0,0,0,0, + &psid ); + ok( r, "failed to allocate sid\n" ); + r = pConvertSidToStringSidA( psid, &str ); + ok( r, "failed to convert sid\n" ); + ok( !strcmp( str, refs[i].refStr ), + "incorrect sid, expected %s, got %s\n", refs[i].refStr, str ); + if( str ) + LocalFree( str ); + if( psid ) + FreeSid( psid ); + + r = pConvertStringSidToSidA( refs[i].refStr, &psid ); + ok( r, "failed to parse sid string\n" ); + pisid = (PISID)psid; + ok( pisid && + !memcmp( pisid->IdentifierAuthority.Value, refs[i].auth.Value, + sizeof(refs[i].auth) ), + "string sid %s didn't parse to expected value\n" + "(got 0x%04x%08lx, expected 0x%04x%08lx)\n", + refs[i].refStr, + MAKEWORD( pisid->IdentifierAuthority.Value[1], + pisid->IdentifierAuthority.Value[0] ), + MAKELONG( MAKEWORD( pisid->IdentifierAuthority.Value[5], + pisid->IdentifierAuthority.Value[4] ), + MAKEWORD( pisid->IdentifierAuthority.Value[3], + pisid->IdentifierAuthority.Value[2] ) ), + MAKEWORD( refs[i].auth.Value[1], refs[i].auth.Value[0] ), + MAKELONG( MAKEWORD( refs[i].auth.Value[5], refs[i].auth.Value[4] ), + MAKEWORD( refs[i].auth.Value[3], refs[i].auth.Value[2] ) ) ); + if( psid ) + LocalFree( psid ); + } } void test_trustee()