From 8cbb9e2b822d96d534400da915a5dc258ab65a9a Mon Sep 17 00:00:00 2001 From: Charles Milette Date: Sat, 6 Mar 2021 03:22:08 -0500 Subject: [PATCH] Feature: Add DetourFindRemotePayload and improve other payload-related methods (#81) Other improvements: - Makes the pcbData parameter in DetourFindPayload and DetourFindPayloadEx optional, so that if an application only needs to search for the presence of a payload, they can ignore the size by passing nullptr. - Makes the pvData parameter in DetourCopyPayloadToProcess const, so that a pointer to a const C++ object can be passed instead of the object needing to be const_casted or being non-const. - Adds DetourCopyPayloadToProcessEx, which has the same interface than DetourCopyPayloadToProcess, but it returns the address of the payload in the remote module, if the program later wants to write to it. - Add payload example and extra unit tests covering new APIs. Fixes #79 Co-authored-by: Charles Milette --- samples/Makefile | 8 ++ samples/payload/Makefile | 59 +++++++++ samples/payload/payload.cpp | 131 +++++++++++++++++++ samples/payload/payloadguid.hpp | 12 ++ samples/payload/payloadtarget.cpp | 60 +++++++++ src/creatwth.cpp | 211 ++++++++++++++++++++++++++---- src/detours.cpp | 16 +++ src/detours.h | 20 ++- src/image.cpp | 13 +- src/modules.cpp | 17 +-- tests/Makefile | 26 ++-- tests/payload.cpp | 25 ++++ tests/payload.h | 25 ++++ tests/process_helpers.cpp | 44 +++++++ tests/process_helpers.h | 37 ++++++ tests/test_module_api.cpp | 146 +++++++++++++++------ 16 files changed, 742 insertions(+), 108 deletions(-) create mode 100644 samples/payload/Makefile create mode 100644 samples/payload/payload.cpp create mode 100644 samples/payload/payloadguid.hpp create mode 100644 samples/payload/payloadtarget.cpp create mode 100644 tests/payload.cpp create mode 100644 tests/payload.h create mode 100644 tests/process_helpers.cpp create mode 100644 tests/process_helpers.h diff --git a/samples/Makefile b/samples/Makefile index c2d5c11..5805d0b 100644 --- a/samples/Makefile +++ b/samples/Makefile @@ -91,6 +91,8 @@ all: @$(MAKE) /NOLOGO /$(MAKEFLAGS) cd "$(MAKEDIR)\dynamic_alloc" @$(MAKE) /NOLOGO /$(MAKEFLAGS) + cd "$(MAKEDIR)\payload" + @$(MAKE) /NOLOGO /$(MAKEFLAGS) cd "$(MAKEDIR)" clean: @@ -156,6 +158,8 @@ clean: @$(MAKE) /NOLOGO /$(MAKEFLAGS) clean cd "$(MAKEDIR)\dynamic_alloc" @$(MAKE) /NOLOGO /$(MAKEFLAGS) clean + cd "$(MAKEDIR)\payload" + @$(MAKE) /NOLOGO /$(MAKEFLAGS) clean cd "$(MAKEDIR)" -rmdir lib32 2>nul -rmdir lib64 2>nul @@ -224,6 +228,8 @@ realclean: @$(MAKE) /NOLOGO /$(MAKEFLAGS) realclean cd "$(MAKEDIR)\dynamic_alloc" @$(MAKE) /NOLOGO /$(MAKEFLAGS) realclean + cd "$(MAKEDIR)\payload" + @$(MAKE) /NOLOGO /$(MAKEFLAGS) realclean cd "$(MAKEDIR)" -rmdir lib32 2>nul -rmdir lib64 2>nul @@ -309,6 +315,8 @@ test: @$(MAKE) /NOLOGO /$(MAKEFLAGS) test cd "$(MAKEDIR)\dynamic_alloc" @$(MAKE) /NOLOGO /$(MAKEFLAGS) test + cd "$(MAKEDIR)\payload" + @$(MAKE) /NOLOGO /$(MAKEFLAGS) test cd "$(MAKEDIR)" ## diff --git a/samples/payload/Makefile b/samples/payload/Makefile new file mode 100644 index 0000000..3271164 --- /dev/null +++ b/samples/payload/Makefile @@ -0,0 +1,59 @@ +############################################################################## +## +## Makefile for Detours Test Programs. +## +## Microsoft Research Detours Package +## +## Copyright (c) Microsoft Corporation. All rights reserved. +## + +!include ..\common.mak + +LIBS=$(LIBS) kernel32.lib +CFLAGS=$(CFLAGS) /EHsc + +all: dirs \ + $(BIND)\payload.exe \ + $(BIND)\payloadtarget.exe + +clean: + -del *~ 2>nul + -del $(BIND)\payload.* 2>nul + -del $(BIND)\payloadtarget.* 2>nul + -rmdir /q /s $(OBJD) 2>nul + +realclean: clean + -rmdir /q /s $(OBJDS) 2>nul + +dirs: + @if not exist $(BIND) mkdir $(BIND) && echo. Created $(BIND) + @if not exist $(OBJD) mkdir $(OBJD) && echo. Created $(OBJD) + +$(OBJD)\payload.obj : payload.cpp + +$(BIND)\payload.exe : $(OBJD)\payload.obj $(DEPS) + link\ + /SUBSYSTEM:CONSOLE\ + $(LINKFLAGS)\ + $(LIBS)\ + /PDB:"$(@R).pdb"\ + /OUT:"$@"\ + $**\ + +$(OBJD)\payloadtarget.obj : payloadtarget.cpp + +$(BIND)\payloadtarget.exe : $(OBJD)\payloadtarget.obj $(DEPS) + link\ + /SUBSYSTEM:CONSOLE\ + $(LINKFLAGS)\ + $(LIBS)\ + /PDB:"$(@R).pdb"\ + /OUT:"$@"\ + $**\ + +############################################################################## + +test: $(BIND)\payload.exe $(BIND)\payloadtarget.exe + $(BIND)\payload.exe + +################################################################# End of File. diff --git a/samples/payload/payload.cpp b/samples/payload/payload.cpp new file mode 100644 index 0000000..5c83768 --- /dev/null +++ b/samples/payload/payload.cpp @@ -0,0 +1,131 @@ +#include +#include +#include +#include + +#include "payloadguid.hpp" + +HANDLE hChildProcess = NULL; +HANDLE hChildThread = NULL; + +__declspec(noreturn) void HandleApiFailure(const char* api) +{ + DWORD lastErr = GetLastError(); + std::cout << "payload.exe: " << api << " failed (" << lastErr << ')' << std::endl; + + if (hChildThread != NULL) + { + CloseHandle(hChildThread); + } + + if (hChildProcess != NULL) + { + TerminateProcess(hChildProcess, 1); + CloseHandle(hChildProcess); + } + + ExitProcess(1); +} + +std::wstring GetProcessFileName(HANDLE process) +{ + DWORD exeLocation_size = MAX_PATH + 1; + + std::wstring exeLocation; + exeLocation.resize(exeLocation_size); + + if (!QueryFullProcessImageNameW(process, 0, &exeLocation[0], &exeLocation_size)) + { + HandleApiFailure("QueryFullProcessImageNameW"); + } + + exeLocation.resize(exeLocation_size); + return exeLocation; +} + +void StartChild() +{ + std::wstring target = GetProcessFileName(GetCurrentProcess()); + target.erase(target.rfind(L'\\') + 1); + target += L"payloadtarget.exe"; + + STARTUPINFOW si = { sizeof(si) }; + PROCESS_INFORMATION pi; + if (!CreateProcessW(target.c_str(), NULL, NULL, NULL, false, + CREATE_SUSPENDED, NULL, NULL, &si, &pi)) + { + HandleApiFailure("CreateProcessW"); + } + + hChildProcess = pi.hProcess; + hChildThread = pi.hThread; +} + +template +volatile T* InjectPayload(HANDLE hProcess, T payload, REFGUID guid) +{ + return static_cast( + DetourCopyPayloadToProcessEx(hProcess,guid, &payload, sizeof(payload))); +} + +int main() +{ + StartChild(); + + // give the child a handle to ourself + HANDLE targetHandleToParent; + if (!DuplicateHandle(GetCurrentProcess(), GetCurrentProcess(), + hChildProcess, &targetHandleToParent, 0, false, DUPLICATE_SAME_ACCESS)) + { + HandleApiFailure("DuplicateHandle"); + } + + if (!InjectPayload(hChildProcess, targetHandleToParent, PARENT_HANDLE_PAYLOAD)) + { + HandleApiFailure("DetourCopyPayloadToProcessEx"); + } + + // inject a payload in ourself containing zero data + // the goal is for the child process to find this payload + // and fill it with random data, to test DetourFindRemotePayload + volatile random_payload_t* payloadAddr = + InjectPayload(GetCurrentProcess(), 0, RANDOM_DATA_PAYLOAD); + if (!payloadAddr) + { + HandleApiFailure("DetourCopyPayloadToProcessEx"); + } + + if (!ResumeThread(hChildThread)) + { + HandleApiFailure("ResumeThread"); + } + + CloseHandle(hChildThread); + hChildThread = NULL; + + if (WaitForSingleObject(hChildProcess, INFINITE) == WAIT_FAILED) + { + HandleApiFailure("WaitForSingleObject"); + } + + DWORD exitCode; + if (!GetExitCodeProcess(hChildProcess, &exitCode)) + { + HandleApiFailure("GetExitCodeProcess"); + } + + // the exit code should match the random data the child process gave us + random_payload_t payload = *payloadAddr; + if (exitCode == payload) + { + std::cout << "Success, exit code (0x" << std::uppercase << std::hex << exitCode + << ") matches payload content (0x" << payload << ')' << std::endl; + return 0; + } + else + { + std::cout << "Error, exit code (0x" << std::uppercase << std::hex << exitCode + << ") does not matches payload content (0x" << payload << ')' << std::endl; + return 1; + } +} \ No newline at end of file diff --git a/samples/payload/payloadguid.hpp b/samples/payload/payloadguid.hpp new file mode 100644 index 0000000..38bac5b --- /dev/null +++ b/samples/payload/payloadguid.hpp @@ -0,0 +1,12 @@ +#pragma once +#include + +// {C2569A74-12FE-4E06-8F02-8DF13E39A266} +const GUID PARENT_HANDLE_PAYLOAD = +{ 0xc2569a74, 0x12fe, 0x4e06, { 0x8f, 0x2, 0x8d, 0xf1, 0x3e, 0x39, 0xa2, 0x66 } }; + +// {CB5230ED-04FA-4C47-B606-AC09B2777601} +const GUID RANDOM_DATA_PAYLOAD = +{ 0xcb5230ed, 0x4fa, 0x4c47, { 0xb6, 0x6, 0xac, 0x9, 0xb2, 0x77, 0x76, 0x1 } }; + +typedef unsigned int random_payload_t; \ No newline at end of file diff --git a/samples/payload/payloadtarget.cpp b/samples/payload/payloadtarget.cpp new file mode 100644 index 0000000..1dc3d0a --- /dev/null +++ b/samples/payload/payloadtarget.cpp @@ -0,0 +1,60 @@ +#define _CRT_RAND_S +#include + +#include +#include +#include + +#include "payloadguid.hpp" + +HANDLE hParent = NULL; + +__declspec(noreturn) void HandleApiFailure(const char* api) +{ + DWORD lastErr = GetLastError(); + std::cout << "payloadtarget.exe: " << api << " failed (" << lastErr << ')' << std::endl; + + if (hParent) + { + CloseHandle(hParent); + } + + ExitProcess(1); +} + +int main() +{ + DWORD payloadSize; + void* payloadAddr = DetourFindPayloadEx(PARENT_HANDLE_PAYLOAD, &payloadSize); + if (!payloadAddr || payloadSize != sizeof(HANDLE)) + { + HandleApiFailure("DetourFindPayloadEx"); + } + + hParent = *static_cast(payloadAddr); + + DWORD randomPayloadSize; + void* randomPayload = DetourFindRemotePayload(hParent, RANDOM_DATA_PAYLOAD, &randomPayloadSize); + if (!randomPayload || randomPayloadSize != sizeof(random_payload_t)) + { + HandleApiFailure("DetourFindRemotePayload"); + } + + random_payload_t randomData; + if (rand_s(&randomData) != 0) + { + HandleApiFailure("rand_s"); + } + + + if (!WriteProcessMemory(hParent, randomPayload, &randomData, sizeof(randomData), NULL)) + { + HandleApiFailure("WriteProcessMemory"); + } + + CloseHandle(hParent); + hParent = NULL; + + // conversion to int return type is potentially undefined + ExitProcess(randomData); +} \ No newline at end of file diff --git a/src/creatwth.cpp b/src/creatwth.cpp index 909fa6b..cb2932a 100644 --- a/src/creatwth.cpp +++ b/src/creatwth.cpp @@ -29,32 +29,32 @@ const GUID DETOUR_EXE_HELPER_GUID = { /* ea0251b9-5cde-41b5-98d0-2af4a26b0fee */ ////////////////////////////////////////////////////////////////////////////// // -// Enumate through modules in the target process. +// Enumerate through modules in the target process. // -static BOOL WINAPI LoadNtHeaderFromProcess(HANDLE hProcess, - HMODULE hModule, - PIMAGE_NT_HEADERS32 pNtHeader) +static PVOID LoadNtHeaderFromProcess(_In_ HANDLE hProcess, + _In_ HMODULE hModule, + _Out_ PIMAGE_NT_HEADERS32 pNtHeader) { + ZeroMemory(pNtHeader, sizeof(*pNtHeader)); PBYTE pbModule = (PBYTE)hModule; if (pbModule == NULL) { SetLastError(ERROR_INVALID_PARAMETER); - return FALSE; + return NULL; } MEMORY_BASIC_INFORMATION mbi; ZeroMemory(&mbi, sizeof(mbi)); if (VirtualQueryEx(hProcess, hModule, &mbi, sizeof(mbi)) == 0) { - return FALSE; + return NULL; } IMAGE_DOS_HEADER idh; - if (!ReadProcessMemory(hProcess, pbModule, &idh, sizeof(idh), NULL)) { DETOUR_TRACE(("ReadProcessMemory(idh@%p..%p) failed: %lx\n", pbModule, pbModule + sizeof(idh), GetLastError())); - return FALSE; + return NULL; } if (idh.e_magic != IMAGE_DOS_SIGNATURE || @@ -62,7 +62,7 @@ static BOOL WINAPI LoadNtHeaderFromProcess(HANDLE hProcess, (DWORD)idh.e_lfanew < sizeof(idh)) { SetLastError(ERROR_BAD_EXE_FORMAT); - return FALSE; + return NULL; } if (!ReadProcessMemory(hProcess, pbModule + idh.e_lfanew, @@ -72,21 +72,27 @@ static BOOL WINAPI LoadNtHeaderFromProcess(HANDLE hProcess, pbModule + idh.e_lfanew + sizeof(*pNtHeader), pbModule, GetLastError())); - return FALSE; + return NULL; } if (pNtHeader->Signature != IMAGE_NT_SIGNATURE) { SetLastError(ERROR_BAD_EXE_FORMAT); - return FALSE; + return NULL; } - return TRUE; + return pbModule + idh.e_lfanew; } -static HMODULE WINAPI EnumerateModulesInProcess(HANDLE hProcess, - HMODULE hModuleLast, - PIMAGE_NT_HEADERS32 pNtHeader) +static HMODULE EnumerateModulesInProcess(_In_ HANDLE hProcess, + _In_opt_ HMODULE hModuleLast, + _Out_ PIMAGE_NT_HEADERS32 pNtHeader, + _Out_opt_ PVOID *pRemoteNtHeader) { + ZeroMemory(pNtHeader, sizeof(*pNtHeader)); + if (pRemoteNtHeader) { + *pRemoteNtHeader = NULL; + } + PBYTE pbLast = (PBYTE)hModuleLast + MM_ALLOCATION_GRANULARITY; MEMORY_BASIC_INFORMATION mbi; @@ -118,13 +124,147 @@ static HMODULE WINAPI EnumerateModulesInProcess(HANDLE hProcess, continue; } - if (LoadNtHeaderFromProcess(hProcess, (HMODULE)pbLast, pNtHeader)) { + PVOID remoteHeader + = LoadNtHeaderFromProcess(hProcess, (HMODULE)pbLast, pNtHeader); + if (remoteHeader) { + if (pRemoteNtHeader) { + *pRemoteNtHeader = remoteHeader; + } + return (HMODULE)pbLast; } } return NULL; } +////////////////////////////////////////////////////////////////////////////// +// +// Find payloads in target process. +// + +static PVOID FindDetourSectionInRemoteModule(_In_ HANDLE hProcess, + _In_ HMODULE hModule, + _In_ const IMAGE_NT_HEADERS32 *pNtHeader, + _In_ PVOID pRemoteNtHeader) +{ + if (pNtHeader->FileHeader.SizeOfOptionalHeader == 0) { + SetLastError(ERROR_EXE_MARKED_INVALID); + return NULL; + } + + PIMAGE_SECTION_HEADER pRemoteSectionHeaders + = (PIMAGE_SECTION_HEADER)((PBYTE)pRemoteNtHeader + + sizeof(pNtHeader->Signature) + + sizeof(pNtHeader->FileHeader) + + pNtHeader->FileHeader.SizeOfOptionalHeader); + + IMAGE_SECTION_HEADER header; + for (DWORD n = 0; n < pNtHeader->FileHeader.NumberOfSections; ++n) { + if (!ReadProcessMemory(hProcess, pRemoteSectionHeaders + n, &header, sizeof(header), NULL)) { + DETOUR_TRACE(("ReadProcessMemory(ish@%p..%p) failed: %lx\n", + pRemoteSectionHeaders + n, + (PBYTE)(pRemoteSectionHeaders + n) + sizeof(header), + GetLastError())); + + return NULL; + } + + if (strcmp((PCHAR)header.Name, ".detour") == 0) { + if (header.VirtualAddress == 0 || + header.SizeOfRawData == 0) { + + break; + } + + SetLastError(NO_ERROR); + return (PBYTE)hModule + header.VirtualAddress; + } + } + + SetLastError(ERROR_EXE_MARKED_INVALID); + return NULL; +} + +static PVOID FindPayloadInRemoteDetourSection(_In_ HANDLE hProcess, + _In_ REFGUID rguid, + _Out_opt_ DWORD *pcbData, + _In_ PVOID pvRemoteDetoursSection) +{ + if (pcbData) { + *pcbData = NULL; + } + + PBYTE pbData = (PBYTE)pvRemoteDetoursSection; + + DETOUR_SECTION_HEADER header; + if (!ReadProcessMemory(hProcess, pbData, &header, sizeof(header), NULL)) { + DETOUR_TRACE(("ReadProcessMemory(dsh@%p..%p) failed: %lx\n", + pbData, + pbData + sizeof(header), + GetLastError())); + return NULL; + } + + if (header.cbHeaderSize < sizeof(DETOUR_SECTION_HEADER) || + header.nSignature != DETOUR_SECTION_HEADER_SIGNATURE) { + SetLastError(ERROR_EXE_MARKED_INVALID); + return NULL; + } + + if (header.nDataOffset == 0) { + header.nDataOffset = header.cbHeaderSize; + } + + for (PVOID pvSection = pbData + header.nDataOffset; pvSection < pbData + header.cbDataSize;) { + DETOUR_SECTION_RECORD section; + if (!ReadProcessMemory(hProcess, pvSection, §ion, sizeof(section), NULL)) { + DETOUR_TRACE(("ReadProcessMemory(dsr@%p..%p) failed: %lx\n", + pvSection, + (PBYTE)pvSection + sizeof(section), + GetLastError())); + return NULL; + } + + if (DetourAreSameGuid(section.guid, rguid)) { + if (pcbData) { + *pcbData = section.cbBytes - sizeof(section); + } + SetLastError(NO_ERROR); + return (DETOUR_SECTION_RECORD *)pvSection + 1; + } + + pvSection = (PBYTE)pvSection + section.cbBytes; + } + + return NULL; +} + +_Success_(return != NULL) +PVOID WINAPI DetourFindRemotePayload(_In_ HANDLE hProcess, + _In_ REFGUID rguid, + _Out_opt_ DWORD *pcbData) +{ + if (hProcess == NULL) { + SetLastError(ERROR_INVALID_HANDLE); + return NULL; + } + + IMAGE_NT_HEADERS32 header; + PVOID pvRemoteHeader; + for (HMODULE hMod = NULL; (hMod = EnumerateModulesInProcess(hProcess, hMod, &header, &pvRemoteHeader)) != NULL;) { + PVOID pvData = FindDetourSectionInRemoteModule(hProcess, hMod, &header, pvRemoteHeader); + if (pvData != NULL) { + pvData = FindPayloadInRemoteDetourSection(hProcess, rguid, pcbData, pvData); + if (pvData != NULL) { + return pvData; + } + } + } + + SetLastError(ERROR_MOD_NOT_FOUND); + return NULL; +} + ////////////////////////////////////////////////////////////////////////////// // // Find a region of memory in which we can create a replacement import table. @@ -550,7 +690,7 @@ BOOL WINAPI DetourUpdateProcessWithDll(_In_ HANDLE hProcess, for (;;) { IMAGE_NT_HEADERS32 inh; - if ((hLast = EnumerateModulesInProcess(hProcess, hLast, &inh)) == NULL) { + if ((hLast = EnumerateModulesInProcess(hProcess, hLast, &inh, NULL)) == NULL) { break; } @@ -872,9 +1012,23 @@ BOOL WINAPI DetourCreateProcessWithDllW(_In_opt_ LPCWSTR lpApplicationName, BOOL WINAPI DetourCopyPayloadToProcess(_In_ HANDLE hProcess, _In_ REFGUID rguid, - _In_reads_bytes_(cbData) PVOID pvData, + _In_reads_bytes_(cbData) LPCVOID pvData, _In_ DWORD cbData) { + return DetourCopyPayloadToProcessEx(hProcess, rguid, pvData, cbData) != NULL; +} + +_Success_(return != NULL) +PVOID WINAPI DetourCopyPayloadToProcessEx(_In_ HANDLE hProcess, + _In_ REFGUID rguid, + _In_reads_bytes_(cbData) LPCVOID pvData, + _In_ DWORD cbData) +{ + if (hProcess == NULL) { + SetLastError(ERROR_INVALID_HANDLE); + return NULL; + } + DWORD cbTotal = (sizeof(IMAGE_DOS_HEADER) + sizeof(IMAGE_NT_HEADERS) + sizeof(IMAGE_SECTION_HEADER) + @@ -886,7 +1040,7 @@ BOOL WINAPI DetourCopyPayloadToProcess(_In_ HANDLE hProcess, MEM_COMMIT, PAGE_READWRITE); if (pbBase == NULL) { DETOUR_TRACE(("VirtualAllocEx(%lx) failed: %lx\n", cbTotal, GetLastError())); - return FALSE; + return NULL; } PBYTE pbTarget = pbBase; @@ -903,7 +1057,7 @@ BOOL WINAPI DetourCopyPayloadToProcess(_In_ HANDLE hProcess, if (!WriteProcessMemory(hProcess, pbTarget, &idh, sizeof(idh), &cbWrote) || cbWrote != sizeof(idh)) { DETOUR_TRACE(("WriteProcessMemory(idh) failed: %lx\n", GetLastError())); - return FALSE; + return NULL; } pbTarget += sizeof(idh); @@ -915,7 +1069,7 @@ BOOL WINAPI DetourCopyPayloadToProcess(_In_ HANDLE hProcess, inh.OptionalHeader.Magic = IMAGE_NT_OPTIONAL_HDR_MAGIC; if (!WriteProcessMemory(hProcess, pbTarget, &inh, sizeof(inh), &cbWrote) || cbWrote != sizeof(inh)) { - return FALSE; + return NULL; } pbTarget += sizeof(inh); @@ -927,7 +1081,7 @@ BOOL WINAPI DetourCopyPayloadToProcess(_In_ HANDLE hProcess, cbData); if (!WriteProcessMemory(hProcess, pbTarget, &ish, sizeof(ish), &cbWrote) || cbWrote != sizeof(ish)) { - return FALSE; + return NULL; } pbTarget += sizeof(ish); @@ -940,7 +1094,7 @@ BOOL WINAPI DetourCopyPayloadToProcess(_In_ HANDLE hProcess, cbData); if (!WriteProcessMemory(hProcess, pbTarget, &dsh, sizeof(dsh), &cbWrote) || cbWrote != sizeof(dsh)) { - return FALSE; + return NULL; } pbTarget += sizeof(dsh); @@ -950,19 +1104,20 @@ BOOL WINAPI DetourCopyPayloadToProcess(_In_ HANDLE hProcess, dsr.guid = rguid; if (!WriteProcessMemory(hProcess, pbTarget, &dsr, sizeof(dsr), &cbWrote) || cbWrote != sizeof(dsr)) { - return FALSE; + return NULL; } pbTarget += sizeof(dsr); if (!WriteProcessMemory(hProcess, pbTarget, pvData, cbData, &cbWrote) || cbWrote != cbData) { - return FALSE; + return NULL; } - pbTarget += cbData; DETOUR_TRACE(("Copied %lx byte payload into target process at %p\n", - cbTotal, pbTarget - cbTotal)); - return TRUE; + cbData, pbTarget)); + + SetLastError(NO_ERROR); + return pbTarget; } static BOOL s_fSearchedForHelper = FALSE; diff --git a/src/detours.cpp b/src/detours.cpp index fa6bd72..10b3634 100644 --- a/src/detours.cpp +++ b/src/detours.cpp @@ -2551,4 +2551,20 @@ BOOL WINAPI DetourVirtualProtectSameExecute(_In_ PVOID pAddress, pAddress, nSize, dwNewProtect, pdwOldProtect); } +BOOL WINAPI DetourAreSameGuid(_In_ REFGUID left, _In_ REFGUID right) +{ + return + left.Data1 == right.Data1 && + left.Data2 == right.Data2 && + left.Data3 == right.Data3 && + left.Data4[0] == right.Data4[0] && + left.Data4[1] == right.Data4[1] && + left.Data4[2] == right.Data4[2] && + left.Data4[3] == right.Data4[3] && + left.Data4[4] == right.Data4[4] && + left.Data4[5] == right.Data4[5] && + left.Data4[6] == right.Data4[6] && + left.Data4[7] == right.Data4[7]; +} + // End of File diff --git a/src/detours.h b/src/detours.h index b6e2196..47dd3ea 100644 --- a/src/detours.h +++ b/src/detours.h @@ -614,13 +614,13 @@ _Readable_bytes_(*pcbData) _Success_(return != NULL) PVOID WINAPI DetourFindPayload(_In_opt_ HMODULE hModule, _In_ REFGUID rguid, - _Out_ DWORD *pcbData); + _Out_opt_ DWORD *pcbData); _Writable_bytes_(*pcbData) _Readable_bytes_(*pcbData) _Success_(return != NULL) PVOID WINAPI DetourFindPayloadEx(_In_ REFGUID rguid, - _Out_ DWORD * pcbData); + _Out_opt_ DWORD *pcbData); DWORD WINAPI DetourGetSizeOfPayloads(_In_opt_ HMODULE hModule); @@ -662,6 +662,11 @@ BOOL WINAPI DetourBinaryClose(_In_ PDETOUR_BINARY pBinary); /////////////////////////////////////////////////// Create Process & Load Dll. // +_Success_(return != NULL) +PVOID WINAPI DetourFindRemotePayload(_In_ HANDLE hProcess, + _In_ REFGUID rguid, + _Out_opt_ DWORD *pcbData); + typedef BOOL (WINAPI *PDETOUR_CREATE_PROCESS_ROUTINEA)( _In_opt_ LPCSTR lpApplicationName, _Inout_opt_ LPSTR lpCommandLine, @@ -828,8 +833,14 @@ BOOL WINAPI DetourUpdateProcessWithDllEx(_In_ HANDLE hProcess, BOOL WINAPI DetourCopyPayloadToProcess(_In_ HANDLE hProcess, _In_ REFGUID rguid, - _In_reads_bytes_(cbData) PVOID pvData, + _In_reads_bytes_(cbData) LPCVOID pvData, _In_ DWORD cbData); +_Success_(return != NULL) +PVOID WINAPI DetourCopyPayloadToProcessEx(_In_ HANDLE hProcess, + _In_ REFGUID rguid, + _In_reads_bytes_(cbData) LPCVOID pvData, + _In_ DWORD cbData); + BOOL WINAPI DetourRestoreAfterWith(VOID); BOOL WINAPI DetourRestoreAfterWithEx(_In_reads_bytes_(cbData) PVOID pvData, _In_ DWORD cbData); @@ -1177,6 +1188,9 @@ BOOL WINAPI DetourVirtualProtectSameExecute(_In_ PVOID pAddress, _In_ SIZE_T nSize, _In_ DWORD dwNewProtect, _Out_ PDWORD pdwOldProtect); + +// Detours must depend only on kernel32.lib, so we cannot use IsEqualGUID +BOOL WINAPI DetourAreSameGuid(_In_ REFGUID left, _In_ REFGUID right); #ifdef __cplusplus } #endif // __cplusplus diff --git a/src/image.cpp b/src/image.cpp index 13c0b22..897df4e 100644 --- a/src/image.cpp +++ b/src/image.cpp @@ -536,18 +536,7 @@ PBYTE CImageData::Find(REFGUID rguid, DWORD *pcbData) continue; } - if (pRecord->guid.Data1 == rguid.Data1 && - pRecord->guid.Data2 == rguid.Data2 && - pRecord->guid.Data3 == rguid.Data3 && - pRecord->guid.Data4[0] == rguid.Data4[0] && - pRecord->guid.Data4[1] == rguid.Data4[1] && - pRecord->guid.Data4[2] == rguid.Data4[2] && - pRecord->guid.Data4[3] == rguid.Data4[3] && - pRecord->guid.Data4[4] == rguid.Data4[4] && - pRecord->guid.Data4[5] == rguid.Data4[5] && - pRecord->guid.Data4[6] == rguid.Data4[6] && - pRecord->guid.Data4[7] == rguid.Data4[7]) { - + if (DetourAreSameGuid(pRecord->guid, rguid)) { *pcbData = cbBytes - sizeof(DETOUR_SECTION_RECORD); return (PBYTE)(pRecord + 1); } diff --git a/src/modules.cpp b/src/modules.cpp index 2c0b522..801743f 100644 --- a/src/modules.cpp +++ b/src/modules.cpp @@ -777,7 +777,7 @@ _Readable_bytes_(*pcbData) _Success_(return != NULL) PVOID WINAPI DetourFindPayload(_In_opt_ HMODULE hModule, _In_ REFGUID rguid, - _Out_ DWORD *pcbData) + _Out_opt_ DWORD *pcbData) { PBYTE pbData = NULL; if (pcbData) { @@ -805,18 +805,7 @@ PVOID WINAPI DetourFindPayload(_In_opt_ HMODULE hModule, for (pbData = pbBeg; pbData < pbEnd;) { DETOUR_SECTION_RECORD *pSection = (DETOUR_SECTION_RECORD *)pbData; - if (pSection->guid.Data1 == rguid.Data1 && - pSection->guid.Data2 == rguid.Data2 && - pSection->guid.Data3 == rguid.Data3 && - pSection->guid.Data4[0] == rguid.Data4[0] && - pSection->guid.Data4[1] == rguid.Data4[1] && - pSection->guid.Data4[2] == rguid.Data4[2] && - pSection->guid.Data4[3] == rguid.Data4[3] && - pSection->guid.Data4[4] == rguid.Data4[4] && - pSection->guid.Data4[5] == rguid.Data4[5] && - pSection->guid.Data4[6] == rguid.Data4[6] && - pSection->guid.Data4[7] == rguid.Data4[7]) { - + if (DetourAreSameGuid(pSection->guid, rguid)) { if (pcbData) { *pcbData = pSection->cbBytes - sizeof(*pSection); } @@ -840,7 +829,7 @@ _Writable_bytes_(*pcbData) _Readable_bytes_(*pcbData) _Success_(return != NULL) PVOID WINAPI DetourFindPayloadEx(_In_ REFGUID rguid, - _Out_ DWORD * pcbData) + _Out_opt_ DWORD *pcbData) { for (HMODULE hMod = NULL; (hMod = DetourEnumerateModules(hMod)) != NULL;) { PVOID pvData; diff --git a/tests/Makefile b/tests/Makefile index 1cc5749..cddcbad 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -11,7 +11,7 @@ ROOT = .. !include ..\samples\common.mak DEPS = $(LIBD)\detours.lib -LIBS=$(LIBS) kernel32.lib +LIBS=$(LIBS) kernel32.lib rpcrt4.lib CFLAGS=$(CFLAGS) /EHsc /DCATCH_CONFIG_NO_WINDOWS_SEH ############################################################################## @@ -30,16 +30,22 @@ $(OBJD)\main.obj : main.cpp $(OBJD)\test_module_api.obj : test_module_api.cpp $(OBJD)\test_image_api.obj : test_image_api.cpp $(OBJD)\corruptor.obj : corruptor.cpp +$(OBJD)\process_helpers.obj : process_helpers.cpp +$(OBJD)\payload.obj : payload.cpp $(BIND)\unittests.exe : $(OBJD)\main.obj \ - $(OBJD)\test_module_api.obj \ - $(OBJD)\test_image_api.obj \ - $(OBJD)\corruptor.obj $(DEPS) + $(OBJD)\test_module_api.obj \ + $(OBJD)\test_image_api.obj \ + $(OBJD)\corruptor.obj \ + $(OBJD)\process_helpers.obj \ + $(OBJD)\payload.obj $(DEPS) cl $(CFLAGS) /Fe$@ /Fd$(@R).pdb \ - $(OBJD)\main.obj \ - $(OBJD)\test_module_api.obj \ - $(OBJD)\test_image_api.obj \ - $(OBJD)\corruptor.obj \ + $(OBJD)\main.obj \ + $(OBJD)\test_module_api.obj \ + $(OBJD)\test_image_api.obj \ + $(OBJD)\corruptor.obj \ + $(OBJD)\process_helpers.obj \ + $(OBJD)\payload.obj \ /link $(LINKFLAGS) $(LIBS) /subsystem:console ############################################################################## @@ -56,8 +62,8 @@ option: ############################################################################## test: all - @cls - $(BIND)\unittests.exe --reporter console --success --durations yes + @cls + $(BIND)\unittests.exe --reporter console --success --durations yes debug: all windbg -o $(BIND)\unittests.exe diff --git a/tests/payload.cpp b/tests/payload.cpp new file mode 100644 index 0000000..1c74a32 --- /dev/null +++ b/tests/payload.cpp @@ -0,0 +1,25 @@ +////////////////////////////////////////////////////////////////////////////// +// +// Test Payload for Detours Module API tests (payload.cpp of unittests.exe) +// +// Microsoft Research Detours Package +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// +#include "payload.h" + +// Define a detours payload for testing. +// +#pragma data_seg(".detour") + +static CPrivateStuff private_stuff = { + DETOUR_SECTION_HEADER_DECLARE(sizeof(CPrivateStuff)), + { + (sizeof(CPrivateStuff) - sizeof(DETOUR_SECTION_HEADER)), + 0, + TEST_PAYLOAD_GUID + }, + "Testing Payload 123" +}; + +#pragma data_seg() diff --git a/tests/payload.h b/tests/payload.h new file mode 100644 index 0000000..4ef3d9d --- /dev/null +++ b/tests/payload.h @@ -0,0 +1,25 @@ +////////////////////////////////////////////////////////////////////////////// +// +// Test Payload for Detours Module API tests (payload.h of unittests.exe) +// +// Microsoft Research Detours Package +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// +#pragma once +#include +#include "windows.h" +#include "detours.h" + +// {85ECA590-6E6A-40FC-BA75-451D96A2A746} +static constexpr GUID TEST_PAYLOAD_GUID = +{ 0x85eca590, 0x6e6a, 0x40fc, { 0xba, 0x75, 0x45, 0x1d, 0x96, 0xa2, 0xa7, 0x46 } }; + +static constexpr std::size_t TEST_PAYLOAD_SIZE = 32; + +struct CPrivateStuff +{ + DETOUR_SECTION_HEADER header; + DETOUR_SECTION_RECORD record; + CHAR szMessage[TEST_PAYLOAD_SIZE]; +}; diff --git a/tests/process_helpers.cpp b/tests/process_helpers.cpp new file mode 100644 index 0000000..b8e84b9 --- /dev/null +++ b/tests/process_helpers.cpp @@ -0,0 +1,44 @@ +////////////////////////////////////////////////////////////////////////////// +// +// Process Test Helpers (process_helpers.cpp of unittests.exe) +// +// Microsoft Research Detours Package +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// +#include "windows.h" +#include "process_helpers.h" + +HRESULT GetProcessFileName(HANDLE process, std::wstring& filename) +{ + filename.resize(MAX_PATH); + + DWORD size = static_cast(filename.size()) + 1; + if (QueryFullProcessImageNameW(process, 0, &filename[0], &size)) + { + filename.resize(size); + return S_OK; + } + else + { + return HRESULT_FROM_WIN32(GetLastError()); + } +} + +HRESULT CreateSuspendedCopy(TerminateOnScopeExit& wrapper) +{ + std::wstring location; + const auto hr = GetProcessFileName(GetCurrentProcess(), location); + if (FAILED(hr)) + { + return hr; + } + + STARTUPINFOW si = { sizeof(si) }; + if (!CreateProcessW(location.c_str(), nullptr, nullptr, nullptr, false, CREATE_SUSPENDED, nullptr, nullptr, &si, &wrapper.information)) + { + return HRESULT_FROM_WIN32(GetLastError()); + } + + return S_OK; +} \ No newline at end of file diff --git a/tests/process_helpers.h b/tests/process_helpers.h new file mode 100644 index 0000000..543c1a1 --- /dev/null +++ b/tests/process_helpers.h @@ -0,0 +1,37 @@ +////////////////////////////////////////////////////// +// +// Process Test Helpers (process_helpers.h of unittests.exe) +// +// Microsoft Research Detours Package +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// +#pragma once +#include +#include + +struct TerminateOnScopeExit +{ + PROCESS_INFORMATION information; + + TerminateOnScopeExit(const TerminateOnScopeExit&) = delete; + TerminateOnScopeExit& operator=(const TerminateOnScopeExit&) = delete; + + ~TerminateOnScopeExit() + { + if (information.hThread) + { + TerminateThread(information.hThread, 0); + CloseHandle(information.hThread); + } + + if (information.hProcess) + { + TerminateProcess(information.hProcess, 0); + CloseHandle(information.hProcess); + } + } +}; + +HRESULT GetProcessFileName(HANDLE process, std::wstring& filename); +HRESULT CreateSuspendedCopy(TerminateOnScopeExit& wrapper); diff --git a/tests/test_module_api.cpp b/tests/test_module_api.cpp index 962c163..f2d1151 100644 --- a/tests/test_module_api.cpp +++ b/tests/test_module_api.cpp @@ -13,6 +13,8 @@ #include "detours.h" #include "corruptor.h" +#include "payload.h" +#include "process_helpers.h" // Expose the image base of the current module for test assertions. // @@ -106,7 +108,7 @@ TEST_CASE("DetourGetContainingModule", "[module]") auto mod = DetourGetContainingModule(GetCommandLineW); REQUIRE( GetLastError() == NO_ERROR ); - REQUIRE( mod == LoadLibrary("kernel32.dll") ); + REQUIRE( mod == LoadLibraryW(L"kernel32.dll") ); } SECTION("Passing own function, results in own HMODULE") @@ -471,42 +473,6 @@ TEST_CASE("DetourEnumerateImports", "[module]") } } -struct CPrivateStuff -{ - DETOUR_SECTION_HEADER header; - DETOUR_SECTION_RECORD record; - CHAR szMessage[32]; -}; - -GUID PayloadGUID -{ /* d9ab8a40-f4cc-11d1-b6d7-006097b010e3 */ - 0xd9ab8a40, - 0xf4cc, - 0x11d1, - {0xb6, 0xd7, 0x00, 0x60, 0x97, 0xb0, 0x10, 0xe3} -}; - -// Define a detours payload for testing. -// -#pragma data_seg(".detour") - -static CPrivateStuff private_stuff = { - DETOUR_SECTION_HEADER_DECLARE(sizeof(CPrivateStuff)), - { - (sizeof(CPrivateStuff) - sizeof(DETOUR_SECTION_HEADER)), - 0, - { /* d9ab8a40-f4cc-11d1-b6d7-006097b010e3 */ - 0xd9ab8a40, - 0xf4cc, - 0x11d1, - {0xb6, 0xd7, 0x00, 0x60, 0x97, 0xb0, 0x10, 0xe3} - } - }, - "Testing Payload 123" -}; - -#pragma data_seg() - TEST_CASE("DetourGetSizeOfPayloads", "[module]") { SECTION("Passing nullptr for module, is successful.") @@ -609,11 +575,11 @@ TEST_CASE("DetourFindPayload", "[module]") HMODULE module {}; DWORD data {}; - auto payload = DetourFindPayload(module, PayloadGUID, &data); + auto payload = DetourFindPayload(module, TEST_PAYLOAD_GUID, &data); REQUIRE( GetLastError() == NO_ERROR ); REQUIRE( payload != nullptr ); - REQUIRE( data == sizeof(CPrivateStuff::szMessage) ); + REQUIRE( data == TEST_PAYLOAD_SIZE ); char* szPayloadMessage = reinterpret_cast(payload); REQUIRE_THAT( szPayloadMessage, Catch::Matchers::Contains("123") ); @@ -643,17 +609,115 @@ TEST_CASE("DetourFindPayloadEx", "[module]") SetLastError(ERROR_INVALID_HANDLE); DWORD data {}; - auto payload = DetourFindPayloadEx(PayloadGUID, &data); + auto payload = DetourFindPayloadEx(TEST_PAYLOAD_GUID, &data); REQUIRE( GetLastError() == NO_ERROR ); REQUIRE( payload != nullptr ); - REQUIRE( data == sizeof(CPrivateStuff::szMessage) ); + REQUIRE( data == TEST_PAYLOAD_SIZE ); char* szPayloadMessage = reinterpret_cast(payload); REQUIRE_THAT( szPayloadMessage, Catch::Matchers::Contains("123") ); } } +TEST_CASE("DetourCopyPayloadToProcessEx", "[module]") +{ + // {44FA1CE0-1DA5-4AFC-946E-F96890C38673} + static constexpr GUID guid = { 0x44fa1ce0, 0x1da5, 0x4afc, { 0x94, 0x6e, 0xf9, 0x68, 0x90, 0xc3, 0x86, 0x73 } }; + static constexpr std::uint32_t data = 0xDEADBEEF; + + SECTION("Passing NULL process handle, results in error") + { + const auto ptr = DetourCopyPayloadToProcessEx(NULL, guid, &data, sizeof(data)); + REQUIRE(GetLastError() == ERROR_INVALID_HANDLE); + REQUIRE(ptr == nullptr); + } + + SECTION("Writing to own process, results in valid pointer") + { + const auto ptr = reinterpret_cast(DetourCopyPayloadToProcessEx(GetCurrentProcess(), guid, &data, sizeof(data))); + REQUIRE(GetLastError() == NO_ERROR); + REQUIRE(*ptr == data); + } + + SECTION("Writing to different process, can be read with ReadProcessMemory") + { + // create a suspended copy of ourself to do things with. + TerminateOnScopeExit process{}; + REQUIRE(SUCCEEDED(CreateSuspendedCopy(process))); + + const auto ptr = DetourCopyPayloadToProcessEx(process.information.hProcess, guid, &data, sizeof(data)); + REQUIRE(GetLastError() == NO_ERROR); + REQUIRE(ptr != nullptr); + + std::uint32_t retrieved_data{}; + REQUIRE(ReadProcessMemory(process.information.hProcess, ptr, &retrieved_data, sizeof(retrieved_data), nullptr)); + REQUIRE(retrieved_data == data); + } +} + +TEST_CASE("DetourFindRemotePayload", "[module]") +{ + SECTION("Passing NULL process handle, results in error") + { + const auto ptr = DetourFindRemotePayload(NULL, TEST_PAYLOAD_GUID, nullptr); + REQUIRE(GetLastError() == ERROR_INVALID_HANDLE); + REQUIRE(ptr == nullptr); + } + + SECTION("Finding null GUID from own process, results in error") + { + const GUID guid{}; + + const auto ptr = DetourFindRemotePayload(GetCurrentProcess(), guid, nullptr); + REQUIRE(GetLastError() == ERROR_MOD_NOT_FOUND); + REQUIRE(ptr == nullptr); + } + + SECTION("Finding null GUID from different process, results in error") + { + // create a suspended copy of ourself to do things with. + TerminateOnScopeExit process{}; + REQUIRE(SUCCEEDED(CreateSuspendedCopy(process))); + + const GUID guid{}; + const auto ptr = DetourFindRemotePayload(process.information.hProcess, guid, nullptr); + REQUIRE(GetLastError() == ERROR_MOD_NOT_FOUND); + REQUIRE(ptr == nullptr); + } + + SECTION("Finding valid GUID from own process, results in valid pointer") + { + DWORD size = 0; + const auto ptr = reinterpret_cast(DetourFindRemotePayload(GetCurrentProcess(), TEST_PAYLOAD_GUID, &size)); + REQUIRE(GetLastError() == NO_ERROR); + REQUIRE(ptr != nullptr); + REQUIRE(size == TEST_PAYLOAD_SIZE); + + char* szPayloadMessage = reinterpret_cast(ptr); + REQUIRE_THAT(szPayloadMessage, Catch::Matchers::Contains("123")); + } + + SECTION("Finding valid GUID from different process, can be read with ReadProcessMemory") + { + // create a suspended copy of ourself to do things with. + TerminateOnScopeExit process{}; + REQUIRE(SUCCEEDED(CreateSuspendedCopy(process))); + + DWORD size = 0; + const auto ptr = DetourFindRemotePayload(process.information.hProcess, TEST_PAYLOAD_GUID, &size); + REQUIRE(GetLastError() == NO_ERROR); + REQUIRE(ptr != nullptr); + REQUIRE(size == TEST_PAYLOAD_SIZE); + + SIZE_T bytesRead = 0; + char szPayloadMessage[TEST_PAYLOAD_SIZE]; + REQUIRE(ReadProcessMemory(process.information.hProcess, ptr, &szPayloadMessage, TEST_PAYLOAD_SIZE, &bytesRead)); + REQUIRE(bytesRead == TEST_PAYLOAD_SIZE); + REQUIRE_THAT(szPayloadMessage, Catch::Matchers::Contains("123")); + } +} + TEST_CASE("DetourRestoreAfterWith", "[module]") { // TODO: Needs to be written.