Fix: Delete payload after finish associated task (microsoft#143)

Delete the payload after finish associated task, and need change the `DETOUR_EXE_RESTORE_GUID`'s value for compatibility with these dlls that compiled by old version Detours.

For example:
- Create a pair of dlls named `dll_createwith_mysocks_32/64.dll`, their function is hooked `CreateProcess` to create any new process with it.
- Create a 32bit exe(named exe_socks) use dll_createwith_mysocks_32.dll by PE import table. So it starts any process will with dll_createwith_mysocks_32/64.dll.

- Now it starts a 32bit exe (named exe_vs), but exe_vs will load a dll (named dll_createwith_myluainject_32/64.dll) that build with detours and their function is hooked `CreateProcess` to create any new process with it.
So if `exe_vs` starts a 64bit exe, `DetourCreateProcessXXX` API will only restore the first IAT which was modified by `dll_createwith_mysocks_32/64.dll`, and `dll_createwith_myluainject_32/64.dll` modified IAT will not restore. 

Because they create payload with the same GUID `DETOUR_EXE_RESTORE_GUID`, Detours will do restore with the first founded payload twice. To fix this we need to delete the payload after the associated task finishes, immediately. If we do this then the payload with GUID `DETOUR_EXE_RESTORE_GUID` can be used by the next dll that complied by Detours.

And for compatible with these dlls that compiled by old version Detours which we don`t have source code to recompile these. So we need change `DETOUR_EXE_RESTORE_GUID`'s value to a new value, so even these dlls can not delete the payload, because we only search the new `DETOUR_EXE_RESTORE_GUID`'s value's payload, the old playload things will not executed by us.

Author: sonyps5201314

src/creatwth.cpp

@@ -1043,6 +1043,11 @@ PVOID WINAPI DetourCopyPayloadToProcessEx(_In_ HANDLE hProcess,
         return NULL;
     }
 
+    // As you can see in the following code,
+    // the memory layout of the payload range "[pbBase, pbBase+cbTotal]" is a PE executable file,
+    // so DetourFreePayload can use "DetourGetContainingModule(Payload pointer)" to get the above "pbBase" pointer,
+    // pbBase: the memory block allocated by VirtualAllocEx will be released in DetourFreePayload by VirtualFree.
+
     PBYTE pbTarget = pbBase;
     IMAGE_DOS_HEADER idh;
     IMAGE_NT_HEADERS inh;
@@ -1176,6 +1181,15 @@ VOID CALLBACK DetourFinishHelperProcess(_In_ HWND,
         rlpDlls = NULL;
     }
 
+    // Note: s_pHelper is allocated as part of injecting the payload in DetourCopyPayloadToProcess(..),
+    // it's a fake section and not data allocated by the system PE loader.
+
+    // Delete the payload after execution to release the memory occupied by it
+    if (s_pHelper != NULL) {
+        DetourFreePayload(s_pHelper);
+        s_pHelper = NULL;
+    }
+
     ExitProcess(Result);
 }
 

src/detours.cpp

@@ -18,6 +18,27 @@
 
 #define NOTHROW
 
+//////////////////////////////////////////////////////////////////////////////
+//
+
+#ifdef _DEBUG
+extern "C" IMAGE_DOS_HEADER __ImageBase;
+int Detour_AssertExprWithFunctionName(int reportType, const char* filename, int linenumber, const char* FunctionName, const char* msg)
+{
+    int nRet = 0;
+    DWORD dwLastError = GetLastError();
+    CHAR szModuleNameWithFunctionName[MAX_PATH * 2];
+    szModuleNameWithFunctionName[0] = 0;
+    GetModuleFileNameA((HMODULE)&__ImageBase, szModuleNameWithFunctionName, ARRAYSIZE(szModuleNameWithFunctionName));
+    StringCchCatNA(szModuleNameWithFunctionName, ARRAYSIZE(szModuleNameWithFunctionName), ",", ARRAYSIZE(szModuleNameWithFunctionName) - strlen(szModuleNameWithFunctionName) - 1);
+    StringCchCatNA(szModuleNameWithFunctionName, ARRAYSIZE(szModuleNameWithFunctionName), FunctionName, ARRAYSIZE(szModuleNameWithFunctionName) - strlen(szModuleNameWithFunctionName) - 1);
+    SetLastError(dwLastError);
+    nRet = _CrtDbgReport(reportType, filename, linenumber, szModuleNameWithFunctionName, msg);
+    SetLastError(dwLastError);
+    return nRet;
+}
+#endif// _DEBUG
+
 //////////////////////////////////////////////////////////////////////////////
 //
 struct _DETOUR_ALIGN

src/detours.h

@@ -48,6 +48,7 @@
 #include <intsafe.h>
 #pragma warning(pop)
 #endif
+#include <crtdbg.h>
 
 // Allow Detours to cleanly compile with the MingW toolchain.
 //
@@ -624,6 +625,7 @@ PVOID WINAPI DetourFindPayloadEx(_In_ REFGUID rguid,
 
 DWORD WINAPI DetourGetSizeOfPayloads(_In_opt_ HMODULE hModule);
 
+BOOL WINAPI DetourFreePayload(_In_ PVOID pvData);
 ///////////////////////////////////////////////// Persistent Binary Functions.
 //
 
@@ -982,6 +984,21 @@ PDETOUR_SYM_INFO DetourLoadImageHlp(VOID);
 #endif
 #define _CRT_STDIO_ARBITRARY_WIDE_SPECIFIERS 1
 
+#ifdef _DEBUG
+
+int Detour_AssertExprWithFunctionName(int reportType, const char* filename, int linenumber, const char* FunctionName, const char* msg);
+
+#define DETOUR_ASSERT_EXPR_WITH_FUNCTION(expr, msg) \
+    (void) ((expr) || \
+    (1 != Detour_AssertExprWithFunctionName(_CRT_ASSERT, __FILE__, __LINE__,__FUNCTION__, msg)) || \
+    (_CrtDbgBreak(), 0))
+
+#define DETOUR_ASSERT(expr) DETOUR_ASSERT_EXPR_WITH_FUNCTION((expr), #expr)
+
+#else// _DEBUG
+#define DETOUR_ASSERT(expr)
+#endif// _DEBUG
+
 #ifndef DETOUR_TRACE
 #if DETOUR_DEBUG
 #define DETOUR_TRACE(x) printf x

src/modules.cpp

@@ -23,8 +23,8 @@
 //////////////////////////////////////////////////////////////////////////////
 //
 const GUID DETOUR_EXE_RESTORE_GUID = {
-    0x2ed7a3ff, 0x3339, 0x4a8d,
-    { 0x80, 0x5c, 0xd4, 0x98, 0x15, 0x3f, 0xc2, 0x8f }};
+    0xbda26f34, 0xbc82, 0x4829,
+    { 0x9e, 0x64, 0x74, 0x2c, 0x4, 0xc8, 0x4f, 0xa0 } };
 
 //////////////////////////////////////////////////////////////////////////////
 //
@@ -843,6 +843,24 @@ PVOID WINAPI DetourFindPayloadEx(_In_ REFGUID rguid,
     return NULL;
 }
 
+BOOL WINAPI DetourFreePayload(_In_ PVOID pvData)
+{
+    BOOL fSucceeded = FALSE;
+
+    // If you have any doubts about the following code, please refer to the comments in DetourCopyPayloadToProcess.
+    HMODULE hModule = DetourGetContainingModule(pvData);
+    DETOUR_ASSERT(hModule != NULL);
+    if (hModule != NULL) {
+        fSucceeded = VirtualFree(hModule, 0, MEM_RELEASE);
+        DETOUR_ASSERT(fSucceeded);
+        if (fSucceeded) {
+            hModule = NULL;
+        }
+    }
+
+    return fSucceeded;
+}
+
 BOOL WINAPI DetourRestoreAfterWithEx(_In_reads_bytes_(cbData) PVOID pvData,
                                      _In_ DWORD cbData)
 {
@@ -889,6 +907,11 @@ BOOL WINAPI DetourRestoreAfterWithEx(_In_reads_bytes_(cbData) PVOID pvData,
         }
         VirtualProtect(pder->pidh, pder->cbidh, dwPermIdh, &dwIgnore);
     }
+    // Delete the payload after successful recovery to prevent repeated restore
+    if (fSucceeded) {
+        DetourFreePayload(pder);
+        pder = NULL;
+    }
     return fSucceeded;
 }