From c029e716b807898090ac66a05b32d28a669078dd Mon Sep 17 00:00:00 2001 From: sonyps5201314 Date: Fri, 4 Sep 2020 15:48:55 +0800 Subject: [PATCH 01/14] fix if the hook is not called at the start of the process, the hook may fail --- samples/common.mak | 2 +- src/creatwth.cpp | 10 +- src/detours.cpp | 392 +++++++++++++++++++++++++++++++++++++++++++-- src/detours.h | 114 +++++++++++++ src/image.cpp | 57 ++++--- src/modules.cpp | 2 +- src/uimports.cpp | 6 +- 7 files changed, 534 insertions(+), 49 deletions(-) diff --git a/samples/common.mak b/samples/common.mak index aa63a156..59b7ac53 100644 --- a/samples/common.mak +++ b/samples/common.mak @@ -23,7 +23,7 @@ CLIB=/MT !ENDIF AFLAGS=/nologo /Zi /c /Fl -CFLAGS=/nologo /Zi $(CLIB) /Gm- /W4 /WX /we4777 /we4800 /Od +CFLAGS=/nologo /Zi $(CLIB) /Gm- /W4 /WX /we4777 /we4800 /Od /D__AUTO_CREATE_DETOUR_HEAP__ !IF $(DETOURS_SOURCE_BROWSING)==1 CFLAGS=$(CFLAGS) /FR diff --git a/src/creatwth.cpp b/src/creatwth.cpp index 96d6b651..aa209db3 100644 --- a/src/creatwth.cpp +++ b/src/creatwth.cpp @@ -991,7 +991,7 @@ VOID CALLBACK DetourFinishHelperProcess(_In_ HWND, goto Cleanup; } - rlpDlls = new NOTHROW LPCSTR [s_pHelper->nDlls]; + rlpDlls = DetourCreateObjectArray(s_pHelper->nDlls); cSize = s_pHelper->cb - sizeof(DETOUR_EXE_HELPER); for (DWORD n = 0; n < s_pHelper->nDlls; n++) { size_t cchDest = 0; @@ -1015,7 +1015,7 @@ VOID CALLBACK DetourFinishHelperProcess(_In_ HWND, Cleanup: if (rlpDlls != NULL) { - delete[] rlpDlls; + DetourDestroyObjectArray(rlpDlls); rlpDlls = NULL; } @@ -1080,7 +1080,7 @@ BOOL WINAPI AllocExeHelper(_Out_ PDETOUR_EXE_HELPER *pHelper, cSize += (DWORD)cchDest + 1; } - Helper = (PDETOUR_EXE_HELPER) new NOTHROW BYTE[sizeof(DETOUR_EXE_HELPER) + cSize]; + Helper = (PDETOUR_EXE_HELPER)DetourCreateObjectArray(sizeof(DETOUR_EXE_HELPER) + cSize); if (Helper == NULL) { goto Cleanup; } @@ -1145,7 +1145,7 @@ BOOL WINAPI AllocExeHelper(_Out_ PDETOUR_EXE_HELPER *pHelper, Cleanup: if (Helper != NULL) { - delete[] (PBYTE)Helper; + DetourDestroyObjectArray((PBYTE)Helper); Helper = NULL; } return Result; @@ -1155,7 +1155,7 @@ static VOID WINAPI FreeExeHelper(PDETOUR_EXE_HELPER *pHelper) { if (*pHelper != NULL) { - delete[] (PBYTE)*pHelper; + DetourDestroyObjectArray((PBYTE)*pHelper); *pHelper = NULL; } } diff --git a/src/detours.cpp b/src/detours.cpp index 34f2458f..efc7a0ef 100644 --- a/src/detours.cpp +++ b/src/detours.cpp @@ -1503,6 +1503,25 @@ struct DetourThread { DetourThread * pNext; HANDLE hThread; + BOOL fCloseThreadHandleOnDestroyDetourThreadObject; +public: + DetourThread() + { + pNext = NULL; + hThread = NULL; + fCloseThreadHandleOnDestroyDetourThreadObject = FALSE; + } + ~DetourThread() + { + if (fCloseThreadHandleOnDestroyDetourThreadObject) + { + if (hThread) + { + CloseHandle(hThread); + hThread = NULL; + } + } + } }; struct DetourOperation @@ -1515,6 +1534,8 @@ struct DetourOperation ULONG dwPerm; }; +static HANDLE s_hHeap = NULL; + static BOOL s_fIgnoreTooSmall = FALSE; static BOOL s_fRetainRegions = FALSE; @@ -1524,6 +1545,47 @@ static PVOID * s_ppPendingError = NULL; static DetourThread * s_pPendingThreads = NULL; static DetourOperation * s_pPendingOperations = NULL; +////////////////////////////////////////////////////////////////////////////// +// +void WINAPI DetourDestroyHeap() +{ + if (s_hHeap) + { + HeapDestroy(s_hHeap); + s_hHeap = NULL; + } +} + +static void DetourDestroyHeap__for__atexit() +{ + DetourDestroyHeap(); +} + +BOOL WINAPI DetourCreateHeap(BOOL fAutoDestroy) +{ + BOOL bResult = FALSE; + if (s_hHeap == NULL) + { + s_hHeap = HeapCreate(0, 0, 0); + assert(s_hHeap); + if (s_hHeap != NULL) + { + if (fAutoDestroy) + { + atexit(DetourDestroyHeap__for__atexit); + } + bResult = TRUE; + } + } + return bResult; +} + +HANDLE WINAPI DetourGetHeap() +{ + assert(s_hHeap); + return s_hHeap; +} + ////////////////////////////////////////////////////////////////////////////// // PVOID WINAPI DetourCodeFromPointer(_In_ PVOID pPointer, @@ -1563,16 +1625,30 @@ PVOID WINAPI DetourSetSystemRegionUpperBound(_In_ PVOID pSystemRegionUpperBound) } LONG WINAPI DetourTransactionBegin() +{ + return DetourTransactionBeginEx(FALSE); +} + +LONG WINAPI DetourTransactionBeginEx(_In_opt_ BOOL fWait) { // Only one transaction is allowed at a time. _Benign_race_begin_ + Check__s_nPendingThreadId: if (s_nPendingThreadId != 0) { + if (fWait) { + Sleep(10); + goto Check__s_nPendingThreadId; + } return ERROR_INVALID_OPERATION; } _Benign_race_end_ // Make sure only one thread can start a transaction. if (InterlockedCompareExchange(&s_nPendingThreadId, (LONG)GetCurrentThreadId(), 0) != 0) { + if (fWait) { + Sleep(10); + goto Check__s_nPendingThreadId; + } return ERROR_INVALID_OPERATION; } @@ -1607,7 +1683,7 @@ LONG WINAPI DetourTransactionAbort() } DetourOperation *n = o->pNext; - delete o; + DetourDestroyObject(o); o = n; } s_pPendingOperations = NULL; @@ -1621,7 +1697,7 @@ LONG WINAPI DetourTransactionAbort() ResumeThread(t->hThread); DetourThread *n = t->pNext; - delete t; + DetourDestroyObject(t); t = n; } s_pPendingThreads = NULL; @@ -1892,7 +1968,7 @@ typedef ULONG_PTR DETOURS_EIP_TYPE; } DetourOperation *n = o->pNext; - delete o; + DetourDestroyObject(o); o = n; } s_pPendingOperations = NULL; @@ -1911,7 +1987,7 @@ typedef ULONG_PTR DETOURS_EIP_TYPE; ResumeThread(t->hThread); DetourThread *n = t->pNext; - delete t; + DetourDestroyObject(t); t = n; } s_pPendingThreads = NULL; @@ -1925,6 +2001,11 @@ typedef ULONG_PTR DETOURS_EIP_TYPE; } LONG WINAPI DetourUpdateThread(_In_ HANDLE hThread) +{ + return DetourUpdateThreadEx(hThread, FALSE); +} + +LONG WINAPI DetourUpdateThreadEx(_In_ HANDLE hThread, _In_opt_ BOOL fCloseThreadHandleOnDestroyDetourThreadObject) { LONG error; @@ -1938,12 +2019,12 @@ LONG WINAPI DetourUpdateThread(_In_ HANDLE hThread) return NO_ERROR; } - DetourThread *t = new NOTHROW DetourThread; + DetourThread *t = DetourCreateObject(); if (t == NULL) { error = ERROR_NOT_ENOUGH_MEMORY; fail: if (t != NULL) { - delete t; + DetourDestroyObject(t); t = NULL; } s_nPendingError = error; @@ -1959,12 +2040,303 @@ LONG WINAPI DetourUpdateThread(_In_ HANDLE hThread) } t->hThread = hThread; + t->fCloseThreadHandleOnDestroyDetourThreadObject = fCloseThreadHandleOnDestroyDetourThreadObject; t->pNext = s_pPendingThreads; s_pPendingThreads = t; return NO_ERROR; } +//Code modified from https://github.com/apriorit/mhook/blob/master/mhook-lib/mhook.c +//========================================================================= +// ntdll definitions + +typedef LONG NTSTATUS; + +typedef LONG KPRIORITY; + +typedef enum _SYSTEM_INFORMATION_CLASS +{ + SystemBasicInformation = 0, + SystemPerformanceInformation = 2, + SystemTimeOfDayInformation = 3, + SystemProcessInformation = 5, + SystemProcessorPerformanceInformation = 8, + SystemHandleInformation = 16, + SystemInterruptInformation = 23, + SystemExceptionInformation = 33, + SystemRegistryQuotaInformation = 37, + SystemLookasideInformation = 45, + SystemProcessIdInformation = 0x58 +} SYSTEM_INFORMATION_CLASS; + +typedef enum _KWAIT_REASON +{ + Executive, + FreePage, + PageIn, + PoolAllocation, + DelayExecution, + Suspended, + UserRequest, + WrExecutive, + WrFreePage, + WrPageIn, + WrPoolAllocation, + WrDelayExecution, + WrSuspended, + WrUserRequest, + WrEventPair, + WrQueue, + WrLpcReceive, + WrLpcReply, + WrVirtualMemory, + WrPageOut, + WrRendezvous, + Spare2, + Spare3, + Spare4, + Spare5, + Spare6, + WrKernel, + MaximumWaitReason +} KWAIT_REASON, *PKWAIT_REASON; + +typedef struct _CLIENT_ID +{ + HANDLE UniqueProcess; + HANDLE UniqueThread; +} CLIENT_ID, *PCLIENT_ID; + +typedef struct _SYSTEM_THREAD_INFORMATION +{ + LARGE_INTEGER KernelTime; + LARGE_INTEGER UserTime; + LARGE_INTEGER CreateTime; + ULONG WaitTime; + PVOID StartAddress; + CLIENT_ID ClientId; + KPRIORITY Priority; + LONG BasePriority; + ULONG ContextSwitches; + ULONG ThreadState; + KWAIT_REASON WaitReason; +} SYSTEM_THREAD_INFORMATION, *PSYSTEM_THREAD_INFORMATION; + +typedef struct _UNICODE_STRING +{ + USHORT Length; + USHORT MaximumLength; + PWSTR Buffer; +} UNICODE_STRING; + +typedef struct _SYSTEM_PROCESS_INFORMATION +{ + ULONG uNext; + ULONG uThreadCount; + LARGE_INTEGER WorkingSetPrivateSize; // since VISTA + ULONG HardFaultCount; // since WIN7 + ULONG NumberOfThreadsHighWatermark; // since WIN7 + ULONGLONG CycleTime; // since WIN7 + LARGE_INTEGER CreateTime; + LARGE_INTEGER UserTime; + LARGE_INTEGER KernelTime; + UNICODE_STRING ImageName; + KPRIORITY BasePriority; + HANDLE uUniqueProcessId; + HANDLE InheritedFromUniqueProcessId; + ULONG HandleCount; + ULONG SessionId; + ULONG_PTR UniqueProcessKey; // since VISTA (requires SystemExtendedProcessInformation) + SIZE_T PeakVirtualSize; + SIZE_T VirtualSize; + ULONG PageFaultCount; + SIZE_T PeakWorkingSetSize; + SIZE_T WorkingSetSize; + SIZE_T QuotaPeakPagedPoolUsage; + SIZE_T QuotaPagedPoolUsage; + SIZE_T QuotaPeakNonPagedPoolUsage; + SIZE_T QuotaNonPagedPoolUsage; + SIZE_T PagefileUsage; + SIZE_T PeakPagefileUsage; + SIZE_T PrivatePageCount; + LARGE_INTEGER ReadOperationCount; + LARGE_INTEGER WriteOperationCount; + LARGE_INTEGER OtherOperationCount; + LARGE_INTEGER ReadTransferCount; + LARGE_INTEGER WriteTransferCount; + LARGE_INTEGER OtherTransferCount; + SYSTEM_THREAD_INFORMATION Threads[1]; +} SYSTEM_PROCESS_INFORMATION, *PSYSTEM_PROCESS_INFORMATION; + +//========================================================================= +// ZwQuerySystemInformation definitions +typedef NTSTATUS(NTAPI* PZwQuerySystemInformation)( + __in SYSTEM_INFORMATION_CLASS SystemInformationClass, + __inout PVOID SystemInformation, + __in ULONG SystemInformationLength, + __out_opt PULONG ReturnLength + ); + +#define STATUS_INFO_LENGTH_MISMATCH ((NTSTATUS)0xC0000004L) +static PZwQuerySystemInformation fnZwQuerySystemInformation = NULL; +//========================================================================= +// Internal function: +// +// Get snapshot of the processes started in the system +//========================================================================= +static BOOL CreateProcessSnapshot(VOID** snapshotContext) +{ + HMODULE hModule = NULL; + ULONG cbBuffer = 1024 * 1024; // 1Mb - default process information buffer size (that's enough in most cases for high-loaded systems) + LPVOID pBuffer = NULL; + NTSTATUS status = 0; + + if (!fnZwQuerySystemInformation) + { + hModule = GetModuleHandle(TEXT("ntdll.dll")); + if (!hModule) + { + return FALSE; + } + fnZwQuerySystemInformation = (PZwQuerySystemInformation)GetProcAddress(hModule, "ZwQuerySystemInformation"); + if (!fnZwQuerySystemInformation) + { + return FALSE; + } + } + + do + { + pBuffer = DetourCreateObjectArray(cbBuffer); + if (pBuffer == NULL) + { + return FALSE; + } + + status = fnZwQuerySystemInformation(SystemProcessInformation, pBuffer, cbBuffer, NULL); + + if (status == STATUS_INFO_LENGTH_MISMATCH) + { + DetourDestroyObjectArray((LPBYTE)pBuffer); + cbBuffer *= 2; + } + else if (status < 0) + { + DetourDestroyObjectArray((LPBYTE)pBuffer); + return FALSE; + } + } while (status == STATUS_INFO_LENGTH_MISMATCH); + + *snapshotContext = pBuffer; + + return TRUE; +} + +//========================================================================= +// Internal function: +// +// Find and return process information from snapshot +//========================================================================= +static PSYSTEM_PROCESS_INFORMATION FindProcess(VOID* snapshotContext, SIZE_T processId) +{ + PSYSTEM_PROCESS_INFORMATION currentProcess = (PSYSTEM_PROCESS_INFORMATION)snapshotContext; + + while (currentProcess != NULL) + { + if (currentProcess->uUniqueProcessId == (HANDLE)processId) + { + break; + } + + if (currentProcess->uNext == 0) + { + currentProcess = NULL; + } + else + { + currentProcess = (PSYSTEM_PROCESS_INFORMATION)(((LPBYTE)currentProcess) + currentProcess->uNext); + } + } + + return currentProcess; +} + +//========================================================================= +// Internal function: +// +// Get current process snapshot and process info +// +//========================================================================= +static BOOL GetCurrentProcessSnapshot(PVOID* snapshot, PSYSTEM_PROCESS_INFORMATION* procInfo) +{ + // get a view of the threads in the system + + if (!CreateProcessSnapshot(snapshot)) + { + DETOUR_TRACE(("detours: can't get process snapshot!")); + return FALSE; + } + + DWORD pid = GetCurrentProcessId(); + + *procInfo = FindProcess(*snapshot, pid); + return TRUE; +} + +//========================================================================= +// Internal function: +// +// Free memory allocated for processes snapshot +//========================================================================= +static VOID CloseProcessSnapshot(VOID* snapshotContext) +{ + DetourDestroyObjectArray((LPBYTE)snapshotContext); +} + +BOOL WINAPI DetourUpdateAllOtherThreads() +{ + LONG bResult = FALSE; + + VOID* procEnumerationCtx = NULL; + PSYSTEM_PROCESS_INFORMATION procInfo = NULL; + + if (GetCurrentProcessSnapshot(&procEnumerationCtx, &procInfo)) + { + DWORD tid = GetCurrentThreadId(); + // go through every thread + for (ULONG threadIdx = 0; threadIdx < procInfo->uThreadCount; threadIdx++) + { + DWORD threadId = (DWORD)(DWORD_PTR)procInfo->Threads[threadIdx].ClientId.UniqueThread; + + if (threadId != tid) + { + HANDLE hThread = OpenThread(THREAD_SUSPEND_RESUME | THREAD_GET_CONTEXT | THREAD_QUERY_INFORMATION | THREAD_SET_CONTEXT, FALSE, threadId); + if (hThread) + { + LONG error = DetourUpdateThreadEx(hThread, TRUE); + assert(error == NO_ERROR); + if (error) + { + DETOUR_TRACE(("DetourUpdateThreadEx failed, error=%d\n", error)); + //Reset s_nPendingError so that subsequent threads can continue to try to call the DetourUpdateThreadEx function + assert(error == s_nPendingError); + //When the console program is terminated, s_nPendingError == ERROR_ACCESS_DENIED may be caused when the SuspendThread is called in DetourUpdateThreadEx + assert(s_nPendingError == ERROR_ACCESS_DENIED); + s_nPendingError = NO_ERROR; + } + } + } + } + + CloseProcessSnapshot(procEnumerationCtx); + + bResult = TRUE; + } + + return bResult; +} + ///////////////////////////////////////////////////////////// Transacted APIs. // LONG WINAPI DetourAttach(_Inout_ PVOID *ppPointer, @@ -2059,7 +2431,7 @@ LONG WINAPI DetourAttachEx(_Inout_ PVOID *ppPointer, *ppRealDetour = pDetour; } - o = new NOTHROW DetourOperation; + o = DetourCreateObject(); if (o == NULL) { error = ERROR_NOT_ENOUGH_MEMORY; fail: @@ -2074,7 +2446,7 @@ LONG WINAPI DetourAttachEx(_Inout_ PVOID *ppPointer, } } if (o != NULL) { - delete o; + DetourDestroyObject(o); o = NULL; } s_ppPendingError = ppPointer; @@ -2355,7 +2727,7 @@ LONG WINAPI DetourDetach(_Inout_ PVOID *ppPointer, return error; } - DetourOperation *o = new NOTHROW DetourOperation; + DetourOperation *o = DetourCreateObject(); if (o == NULL) { error = ERROR_NOT_ENOUGH_MEMORY; fail: @@ -2363,7 +2735,7 @@ LONG WINAPI DetourDetach(_Inout_ PVOID *ppPointer, DETOUR_BREAK(); stop: if (o != NULL) { - delete o; + DetourDestroyObject(o); o = NULL; } s_ppPendingError = ppPointer; diff --git a/src/detours.h b/src/detours.h index c08b6f19..6fc1b9e6 100644 --- a/src/detours.h +++ b/src/detours.h @@ -47,6 +47,8 @@ #include #pragma warning(pop) #endif +#include +#include // From winerror.h, as this error isn't found in some SDKs: // @@ -539,14 +541,26 @@ typedef BOOL (CALLBACK *PF_DETOUR_IMPORT_FUNC_CALLBACK_EX)(_In_opt_ PVOID pConte typedef VOID * PDETOUR_BINARY; typedef VOID * PDETOUR_LOADED_BINARY; +//////////////////////////////////////////////////////////// Memory management APIs. +BOOL WINAPI DetourCreateHeap(BOOL fAutoDestroy); +HANDLE WINAPI DetourGetHeap(); +void WINAPI DetourDestroyHeap(); +#ifdef __AUTO_CREATE_DETOUR_HEAP__ +static BOOL s_fAlreadyCreatedDetourHeap = DetourCreateHeap(TRUE); +#endif + //////////////////////////////////////////////////////////// Transaction APIs. // LONG WINAPI DetourTransactionBegin(VOID); +LONG WINAPI DetourTransactionBeginEx(_In_opt_ BOOL fWait /*= TRUE*/); LONG WINAPI DetourTransactionAbort(VOID); LONG WINAPI DetourTransactionCommit(VOID); LONG WINAPI DetourTransactionCommitEx(_Out_opt_ PVOID **pppFailedPointer); +//DetourUpdateThread is no longer recommended to be called by users, please use DetourUpdateAllOtherThreads for safer HOOK LONG WINAPI DetourUpdateThread(_In_ HANDLE hThread); +LONG WINAPI DetourUpdateThreadEx(_In_ HANDLE hThread, _In_opt_ BOOL fCloseThreadHandleOnDestroyDetourThreadObject /*= TRUE*/); +BOOL WINAPI DetourUpdateAllOtherThreads(); LONG WINAPI DetourAttach(_Inout_ PVOID *ppPointer, _In_ PVOID pDetour); @@ -1118,6 +1132,106 @@ BOOL WINAPI DetourVirtualProtectSameExecute(_In_ PVOID pAddress, } #endif // __cplusplus +//////////////////////////////////////////////////////////// Memory management APIs. +template +T* DetourCreateObject() +{ + T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T)); + assert(p); + if (!p) + { + return p; + } + + ::new(p) T; + + return p; +} +template +T* DetourCreateObject(P1 p1) +{ + T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T)); + assert(p); + if (!p) + { + return p; + } + + ::new(p) T(p1); + + return p; +} +template +T* DetourCreateObject(P1 p1, P2 p2) +{ + T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T)); + assert(p); + if (!p) + { + return p; + } + + ::new(p) T(p1, p2); + + return p; +} +template +void DetourDestroyObject(T* p) +{ + size_t MemSize = HeapSize(DetourGetHeap(), 0, p); + assert(MemSize == sizeof(T)); + if (MemSize != sizeof(T)) + { + return; + } + + p->~T(); + + HeapFree(DetourGetHeap(), 0, p); + p = NULL; +} +template +T* DetourCreateObjectArray(size_t _Size) +{ + T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T) * _Size); + assert(p); + if (!p) + { + return p; + } + + for (size_t i = 0; i < _Size; i++) + { + ::new(&p[i]) T; + } + + return p; +} +template +void DetourDestroyObjectArray(T* p) +{ + size_t MemSize = HeapSize(DetourGetHeap(), 0, p); + assert(MemSize > 0); + if (MemSize == 0) + { + return; + } + size_t _Size = MemSize / sizeof(T); + assert(_Size > 0); + if (_Size == 0) + { + return; + } + + for (size_t i = 0; i < _Size; i++) + { + (&p[i])->~T(); + } + + HeapFree(DetourGetHeap(), 0, (LPVOID)p); + p = NULL; +} + ////////////////////////////////////////////////////////////////////////////// #define MM_ALLOCATION_GRANULARITY 0x10000 diff --git a/src/image.cpp b/src/image.cpp index 96282d21..1ba75a5d 100644 --- a/src/image.cpp +++ b/src/image.cpp @@ -321,7 +321,7 @@ static LPCSTR DuplicateString(_In_ LPCSTR pszIn) return NULL; } - PCHAR pszOut = new NOTHROW CHAR [cch + 1]; + PCHAR pszOut = DetourCreateObjectArray(cch + 1); if (pszOut == NULL) { SetLastError(ERROR_OUTOFMEMORY); return NULL; @@ -329,7 +329,7 @@ static LPCSTR DuplicateString(_In_ LPCSTR pszIn) hr = StringCchCopyA(pszOut, cch + 1, pszIn); if (FAILED(hr)) { - delete[] pszOut; + DetourDestroyObjectArray(pszOut); return NULL; } @@ -339,7 +339,7 @@ static LPCSTR DuplicateString(_In_ LPCSTR pszIn) static VOID ReleaseString(_In_opt_ LPCSTR psz) { if (psz != NULL) { - delete[] psz; + DetourDestroyObjectArray(psz); } } @@ -364,20 +364,20 @@ CImageImportFile::CImageImportFile() CImageImportFile::~CImageImportFile() { if (m_pNextFile) { - delete m_pNextFile; + DetourDestroyObject(m_pNextFile); m_pNextFile = NULL; } if (m_pImportNames) { - delete[] m_pImportNames; + DetourDestroyObjectArray(m_pImportNames); m_pImportNames = NULL; m_nImportNames = 0; } if (m_pszName) { - delete[] m_pszName; + DetourDestroyObjectArray(m_pszName); m_pszName = NULL; } if (m_pszOrig) { - delete[] m_pszOrig; + DetourDestroyObjectArray(m_pszOrig); m_pszOrig = NULL; } } @@ -394,11 +394,11 @@ CImageImportName::CImageImportName() CImageImportName::~CImageImportName() { if (m_pszName) { - delete[] m_pszName; + DetourDestroyObjectArray(m_pszName); m_pszName = NULL; } if (m_pszOrig) { - delete[] m_pszOrig; + DetourDestroyObjectArray(m_pszOrig); m_pszOrig = NULL; } } @@ -420,7 +420,7 @@ CImageData::~CImageData() m_pbData = NULL; } if (m_pbData) { - delete[] m_pbData; + DetourDestroyObjectArray(m_pbData); m_pbData = NULL; } m_cbData = 0; @@ -435,7 +435,7 @@ BOOL CImageData::SizeTo(DWORD cbData) return TRUE; } - PBYTE pbNew = new NOTHROW BYTE [cbData]; + PBYTE pbNew = DetourCreateObjectArray(cbData); if (pbNew == NULL) { SetLastError(ERROR_OUTOFMEMORY); return FALSE; @@ -444,7 +444,7 @@ BOOL CImageData::SizeTo(DWORD cbData) if (m_pbData) { CopyMemory(pbNew, m_pbData, m_cbData); if (m_cbAlloc > 0) { - delete[] m_pbData; + DetourDestroyObjectArray(m_pbData); } m_pbData = NULL; } @@ -798,13 +798,13 @@ CImage::~CImage() BOOL CImage::Close() { if (m_pImportFiles) { - delete m_pImportFiles; + DetourDestroyObject(m_pImportFiles); m_pImportFiles = NULL; m_nImportFiles = 0; } if (m_pImageData) { - delete m_pImageData; + DetourDestroyObject(m_pImageData); m_pImageData = NULL; } @@ -819,7 +819,7 @@ BOOL CImage::Close() } if (m_pbOutputBuffer) { - delete[] m_pbOutputBuffer; + DetourDestroyObjectArray(m_pbOutputBuffer); m_pbOutputBuffer = NULL; m_cbOutputBuffer = 0; } @@ -878,7 +878,7 @@ BOOL CImage::SizeOutputBuffer(DWORD cbData) } cbData = FileAlign(cbData); - PBYTE pOutput = new NOTHROW BYTE [cbData]; + PBYTE pOutput = DetourCreateObjectArray(cbData); if (pOutput == NULL) { SetLastError(ERROR_OUTOFMEMORY); return FALSE; @@ -887,7 +887,7 @@ BOOL CImage::SizeOutputBuffer(DWORD cbData) if (m_pbOutputBuffer) { CopyMemory(pOutput, m_pbOutputBuffer, m_cbOutputBuffer); - delete[] m_pbOutputBuffer; + DetourDestroyObjectArray(m_pbOutputBuffer); m_pbOutputBuffer = NULL; } @@ -1159,7 +1159,7 @@ BOOL CImage::Read(HANDLE hFile) goto fail; } - CImageImportFile *pImportFile = new NOTHROW CImageImportFile; + CImageImportFile *pImportFile = DetourCreateObject(); if (pImportFile == NULL) { SetLastError(ERROR_OUTOFMEMORY); goto fail; @@ -1219,7 +1219,7 @@ BOOL CImage::Read(HANDLE hFile) if (pAddrThunk && nNames) { pImportFile->m_nImportNames = nNames; - pImportFile->m_pImportNames = new NOTHROW CImageImportName [nNames]; + pImportFile->m_pImportNames = DetourCreateObjectArray(nNames); if (pImportFile->m_pImportNames == NULL) { SetLastError(ERROR_OUTOFMEMORY); goto fail; @@ -1329,7 +1329,7 @@ BOOL CImage::Read(HANDLE hFile) } } - m_pImageData = new NOTHROW CImageData(pbData, cbData); + m_pImageData = DetourCreateObject(pbData, cbData); if (m_pImageData == NULL) { SetLastError(ERROR_OUTOFMEMORY); } @@ -1406,7 +1406,7 @@ BOOL CImage::CheckImportsNeeded(DWORD *pnTables, DWORD *pnThunks, DWORD *pnChars // CImageImportFile * CImage::NewByway(_In_ LPCSTR pszName) { - CImageImportFile *pImportFile = new NOTHROW CImageImportFile; + CImageImportFile *pImportFile = DetourCreateObject(); if (pImportFile == NULL) { SetLastError(ERROR_OUTOFMEMORY); goto fail; @@ -1431,7 +1431,7 @@ CImageImportFile * CImage::NewByway(_In_ LPCSTR pszName) fail: if (pImportFile) { - delete pImportFile; + DetourDestroyObject(pImportFile); pImportFile = NULL; } return NULL; @@ -1493,7 +1493,7 @@ BOOL CImage::EditImports(PVOID pContext, else { // Delete Byway *ppLastFile = pImportFile->m_pNextFile; pImportFile->m_pNextFile = NULL; - delete pImportFile; + DetourDestroyObject(pImportFile); m_nImportFiles--; continue; // Retry after delete. } @@ -1556,7 +1556,7 @@ BOOL CImage::EditImports(PVOID pContext, pImportName->m_nOrdinal = nOrdinal; if (pImportName->m_pszName != NULL) { - delete[] pImportName->m_pszName; + DetourDestroyObjectArray(pImportName->m_pszName); pImportName->m_pszName = NULL; } } @@ -1693,7 +1693,7 @@ BOOL CImage::Write(HANDLE hFile) m_nNextFileAddr = Max(m_SectionHeaders[n].PointerToRawData + m_SectionHeaders[n].SizeOfRawData, m_nNextFileAddr); - // Old images have VirtualSize == 0 as a matter of course, e.g. NT 3.1. + // Old images have VirtualSize == 0 as a matter of course, e.g. NT 3.1. // In which case, use SizeOfRawData instead. m_nNextVirtAddr = Max(m_SectionHeaders[n].VirtualAddress + (m_SectionHeaders[n].Misc.VirtualSize @@ -2040,15 +2040,14 @@ BOOL CImage::Write(HANDLE hFile) // PDETOUR_BINARY WINAPI DetourBinaryOpen(_In_ HANDLE hFile) { - Detour::CImage *pImage = new NOTHROW - Detour::CImage; + Detour::CImage *pImage = DetourCreateObject(); if (pImage == NULL) { SetLastError(ERROR_OUTOFMEMORY); return FALSE; } if (!pImage->Read(hFile)) { - delete pImage; + DetourDestroyObject(pImage); return FALSE; } @@ -2216,7 +2215,7 @@ BOOL WINAPI DetourBinaryClose(_In_ PDETOUR_BINARY pBinary) } BOOL bSuccess = pImage->Close(); - delete pImage; + DetourDestroyObject(pImage); pImage = NULL; return bSuccess; diff --git a/src/modules.cpp b/src/modules.cpp index ade78c97..e4f94faf 100644 --- a/src/modules.cpp +++ b/src/modules.cpp @@ -224,7 +224,7 @@ PVOID WINAPI DetourFindFunction(_In_ LPCSTR pszModule, // and have to convert it to a wrapped [code pointer, global pointer]. // PPLABEL_DESCRIPTOR pldEntry = (PPLABEL_DESCRIPTOR)DetourGetEntryPoint(hModule); - PPLABEL_DESCRIPTOR pldSymbol = new PLABEL_DESCRIPTOR; + PPLABEL_DESCRIPTOR pldSymbol = DetourCreateObject(); pldSymbol->EntryPoint = symbol.Address; pldSymbol->GlobalPointer = pldEntry->GlobalPointer; diff --git a/src/uimports.cpp b/src/uimports.cpp index 4b981a28..1d2117a2 100644 --- a/src/uimports.cpp +++ b/src/uimports.cpp @@ -40,7 +40,7 @@ static BOOL UPDATE_IMPORTS_XX(HANDLE hProcess, finish: if (pbNew != NULL) { - delete[] pbNew; + DetourDestroyObjectArray(pbNew); pbNew = NULL; } return fSucceeded; @@ -125,9 +125,9 @@ static BOOL UPDATE_IMPORTS_XX(HANDLE hProcess, _Analysis_assume_(cbNew > sizeof(IMAGE_IMPORT_DESCRIPTOR) * (nDlls + nOldDlls) + sizeof(DWORD_XX) * 4 * nDlls); - pbNew = new BYTE [cbNew]; + pbNew = DetourCreateObjectArray(cbNew); if (pbNew == NULL) { - DETOUR_TRACE(("new BYTE [cbNew] failed.\n")); + DETOUR_TRACE(("DetourCreateObjectArray(cbNew) failed.\n")); goto finish; } ZeroMemory(pbNew, cbNew); From 1b652fcc0f4e9eafc68ec3b6e59418543eeb6bc2 Mon Sep 17 00:00:00 2001 From: sonyps5201314 Date: Sun, 6 Sep 2020 22:04:31 +0800 Subject: [PATCH 02/14] The code format is adapted to the Detours code style --- src/detours.cpp | 493 +++++++++++++++++++++++------------------------- src/detours.h | 123 ++++++------ 2 files changed, 292 insertions(+), 324 deletions(-) diff --git a/src/detours.cpp b/src/detours.cpp index efc7a0ef..82cca96b 100644 --- a/src/detours.cpp +++ b/src/detours.cpp @@ -863,7 +863,7 @@ struct _DETOUR_TRAMPOLINE // An ARM64 instruction is 4 bytes long. // // The overwrite is always composed of 3 instructions (12 bytes) which perform an indirect jump - // using _DETOUR_TRAMPOLINE::pbDetour as the address holding the target location. + // using _DETOUR_TRAMPOLINE::pbDetour as the address holding the target location. // // Copied instructions can expand. // @@ -1503,25 +1503,23 @@ struct DetourThread { DetourThread * pNext; HANDLE hThread; - BOOL fCloseThreadHandleOnDestroyDetourThreadObject; + BOOL fCloseThreadHandleOnDestroyDetourThreadObject; public: - DetourThread() - { - pNext = NULL; - hThread = NULL; - fCloseThreadHandleOnDestroyDetourThreadObject = FALSE; - } - ~DetourThread() - { - if (fCloseThreadHandleOnDestroyDetourThreadObject) - { - if (hThread) - { - CloseHandle(hThread); - hThread = NULL; - } - } - } + DetourThread() + { + pNext = NULL; + hThread = NULL; + fCloseThreadHandleOnDestroyDetourThreadObject = FALSE; + } + ~DetourThread() + { + if (fCloseThreadHandleOnDestroyDetourThreadObject) { + if (hThread) { + CloseHandle(hThread); + hThread = NULL; + } + } + } }; struct DetourOperation @@ -1549,41 +1547,37 @@ static DetourOperation * s_pPendingOperations = NULL; // void WINAPI DetourDestroyHeap() { - if (s_hHeap) - { - HeapDestroy(s_hHeap); - s_hHeap = NULL; - } + if (s_hHeap) { + HeapDestroy(s_hHeap); + s_hHeap = NULL; + } } static void DetourDestroyHeap__for__atexit() { - DetourDestroyHeap(); + DetourDestroyHeap(); } BOOL WINAPI DetourCreateHeap(BOOL fAutoDestroy) { - BOOL bResult = FALSE; - if (s_hHeap == NULL) - { - s_hHeap = HeapCreate(0, 0, 0); - assert(s_hHeap); - if (s_hHeap != NULL) - { - if (fAutoDestroy) - { - atexit(DetourDestroyHeap__for__atexit); - } - bResult = TRUE; - } - } - return bResult; + BOOL bResult = FALSE; + if (s_hHeap == NULL) { + s_hHeap = HeapCreate(0, 0, 0); + assert(s_hHeap); + if (s_hHeap != NULL) { + if (fAutoDestroy) { + atexit(DetourDestroyHeap__for__atexit); + } + bResult = TRUE; + } + } + return bResult; } HANDLE WINAPI DetourGetHeap() { - assert(s_hHeap); - return s_hHeap; + assert(s_hHeap); + return s_hHeap; } ////////////////////////////////////////////////////////////////////////////// @@ -1633,22 +1627,22 @@ LONG WINAPI DetourTransactionBeginEx(_In_opt_ BOOL fWait) { // Only one transaction is allowed at a time. _Benign_race_begin_ - Check__s_nPendingThreadId: + Check__s_nPendingThreadId: if (s_nPendingThreadId != 0) { - if (fWait) { - Sleep(10); - goto Check__s_nPendingThreadId; - } + if (fWait) { + Sleep(10); + goto Check__s_nPendingThreadId; + } return ERROR_INVALID_OPERATION; } _Benign_race_end_ // Make sure only one thread can start a transaction. if (InterlockedCompareExchange(&s_nPendingThreadId, (LONG)GetCurrentThreadId(), 0) != 0) { - if (fWait) { - Sleep(10); - goto Check__s_nPendingThreadId; - } + if (fWait) { + Sleep(10); + goto Check__s_nPendingThreadId; + } return ERROR_INVALID_OPERATION; } @@ -2040,7 +2034,7 @@ LONG WINAPI DetourUpdateThreadEx(_In_ HANDLE hThread, _In_opt_ BOOL fCloseThread } t->hThread = hThread; - t->fCloseThreadHandleOnDestroyDetourThreadObject = fCloseThreadHandleOnDestroyDetourThreadObject; + t->fCloseThreadHandleOnDestroyDetourThreadObject = fCloseThreadHandleOnDestroyDetourThreadObject; t->pNext = s_pPendingThreads; s_pPendingThreads = t; @@ -2057,126 +2051,126 @@ typedef LONG KPRIORITY; typedef enum _SYSTEM_INFORMATION_CLASS { - SystemBasicInformation = 0, - SystemPerformanceInformation = 2, - SystemTimeOfDayInformation = 3, - SystemProcessInformation = 5, - SystemProcessorPerformanceInformation = 8, - SystemHandleInformation = 16, - SystemInterruptInformation = 23, - SystemExceptionInformation = 33, - SystemRegistryQuotaInformation = 37, - SystemLookasideInformation = 45, - SystemProcessIdInformation = 0x58 + SystemBasicInformation = 0, + SystemPerformanceInformation = 2, + SystemTimeOfDayInformation = 3, + SystemProcessInformation = 5, + SystemProcessorPerformanceInformation = 8, + SystemHandleInformation = 16, + SystemInterruptInformation = 23, + SystemExceptionInformation = 33, + SystemRegistryQuotaInformation = 37, + SystemLookasideInformation = 45, + SystemProcessIdInformation = 0x58 } SYSTEM_INFORMATION_CLASS; typedef enum _KWAIT_REASON { - Executive, - FreePage, - PageIn, - PoolAllocation, - DelayExecution, - Suspended, - UserRequest, - WrExecutive, - WrFreePage, - WrPageIn, - WrPoolAllocation, - WrDelayExecution, - WrSuspended, - WrUserRequest, - WrEventPair, - WrQueue, - WrLpcReceive, - WrLpcReply, - WrVirtualMemory, - WrPageOut, - WrRendezvous, - Spare2, - Spare3, - Spare4, - Spare5, - Spare6, - WrKernel, - MaximumWaitReason -} KWAIT_REASON, *PKWAIT_REASON; + Executive, + FreePage, + PageIn, + PoolAllocation, + DelayExecution, + Suspended, + UserRequest, + WrExecutive, + WrFreePage, + WrPageIn, + WrPoolAllocation, + WrDelayExecution, + WrSuspended, + WrUserRequest, + WrEventPair, + WrQueue, + WrLpcReceive, + WrLpcReply, + WrVirtualMemory, + WrPageOut, + WrRendezvous, + Spare2, + Spare3, + Spare4, + Spare5, + Spare6, + WrKernel, + MaximumWaitReason +} KWAIT_REASON, * PKWAIT_REASON; typedef struct _CLIENT_ID { - HANDLE UniqueProcess; - HANDLE UniqueThread; -} CLIENT_ID, *PCLIENT_ID; + HANDLE UniqueProcess; + HANDLE UniqueThread; +} CLIENT_ID, * PCLIENT_ID; typedef struct _SYSTEM_THREAD_INFORMATION { - LARGE_INTEGER KernelTime; - LARGE_INTEGER UserTime; - LARGE_INTEGER CreateTime; - ULONG WaitTime; - PVOID StartAddress; - CLIENT_ID ClientId; - KPRIORITY Priority; - LONG BasePriority; - ULONG ContextSwitches; - ULONG ThreadState; - KWAIT_REASON WaitReason; -} SYSTEM_THREAD_INFORMATION, *PSYSTEM_THREAD_INFORMATION; + LARGE_INTEGER KernelTime; + LARGE_INTEGER UserTime; + LARGE_INTEGER CreateTime; + ULONG WaitTime; + PVOID StartAddress; + CLIENT_ID ClientId; + KPRIORITY Priority; + LONG BasePriority; + ULONG ContextSwitches; + ULONG ThreadState; + KWAIT_REASON WaitReason; +} SYSTEM_THREAD_INFORMATION, * PSYSTEM_THREAD_INFORMATION; typedef struct _UNICODE_STRING { - USHORT Length; - USHORT MaximumLength; - PWSTR Buffer; + USHORT Length; + USHORT MaximumLength; + PWSTR Buffer; } UNICODE_STRING; typedef struct _SYSTEM_PROCESS_INFORMATION { - ULONG uNext; - ULONG uThreadCount; - LARGE_INTEGER WorkingSetPrivateSize; // since VISTA - ULONG HardFaultCount; // since WIN7 - ULONG NumberOfThreadsHighWatermark; // since WIN7 - ULONGLONG CycleTime; // since WIN7 - LARGE_INTEGER CreateTime; - LARGE_INTEGER UserTime; - LARGE_INTEGER KernelTime; - UNICODE_STRING ImageName; - KPRIORITY BasePriority; - HANDLE uUniqueProcessId; - HANDLE InheritedFromUniqueProcessId; - ULONG HandleCount; - ULONG SessionId; - ULONG_PTR UniqueProcessKey; // since VISTA (requires SystemExtendedProcessInformation) - SIZE_T PeakVirtualSize; - SIZE_T VirtualSize; - ULONG PageFaultCount; - SIZE_T PeakWorkingSetSize; - SIZE_T WorkingSetSize; - SIZE_T QuotaPeakPagedPoolUsage; - SIZE_T QuotaPagedPoolUsage; - SIZE_T QuotaPeakNonPagedPoolUsage; - SIZE_T QuotaNonPagedPoolUsage; - SIZE_T PagefileUsage; - SIZE_T PeakPagefileUsage; - SIZE_T PrivatePageCount; - LARGE_INTEGER ReadOperationCount; - LARGE_INTEGER WriteOperationCount; - LARGE_INTEGER OtherOperationCount; - LARGE_INTEGER ReadTransferCount; - LARGE_INTEGER WriteTransferCount; - LARGE_INTEGER OtherTransferCount; - SYSTEM_THREAD_INFORMATION Threads[1]; -} SYSTEM_PROCESS_INFORMATION, *PSYSTEM_PROCESS_INFORMATION; + ULONG uNext; + ULONG uThreadCount; + LARGE_INTEGER WorkingSetPrivateSize; // since VISTA + ULONG HardFaultCount; // since WIN7 + ULONG NumberOfThreadsHighWatermark; // since WIN7 + ULONGLONG CycleTime; // since WIN7 + LARGE_INTEGER CreateTime; + LARGE_INTEGER UserTime; + LARGE_INTEGER KernelTime; + UNICODE_STRING ImageName; + KPRIORITY BasePriority; + HANDLE uUniqueProcessId; + HANDLE InheritedFromUniqueProcessId; + ULONG HandleCount; + ULONG SessionId; + ULONG_PTR UniqueProcessKey; // since VISTA (requires SystemExtendedProcessInformation) + SIZE_T PeakVirtualSize; + SIZE_T VirtualSize; + ULONG PageFaultCount; + SIZE_T PeakWorkingSetSize; + SIZE_T WorkingSetSize; + SIZE_T QuotaPeakPagedPoolUsage; + SIZE_T QuotaPagedPoolUsage; + SIZE_T QuotaPeakNonPagedPoolUsage; + SIZE_T QuotaNonPagedPoolUsage; + SIZE_T PagefileUsage; + SIZE_T PeakPagefileUsage; + SIZE_T PrivatePageCount; + LARGE_INTEGER ReadOperationCount; + LARGE_INTEGER WriteOperationCount; + LARGE_INTEGER OtherOperationCount; + LARGE_INTEGER ReadTransferCount; + LARGE_INTEGER WriteTransferCount; + LARGE_INTEGER OtherTransferCount; + SYSTEM_THREAD_INFORMATION Threads[1]; +} SYSTEM_PROCESS_INFORMATION, * PSYSTEM_PROCESS_INFORMATION; //========================================================================= // ZwQuerySystemInformation definitions typedef NTSTATUS(NTAPI* PZwQuerySystemInformation)( - __in SYSTEM_INFORMATION_CLASS SystemInformationClass, - __inout PVOID SystemInformation, - __in ULONG SystemInformationLength, - __out_opt PULONG ReturnLength - ); + __in SYSTEM_INFORMATION_CLASS SystemInformationClass, + __inout PVOID SystemInformation, + __in ULONG SystemInformationLength, + __out_opt PULONG ReturnLength + ); #define STATUS_INFO_LENGTH_MISMATCH ((NTSTATUS)0xC0000004L) static PZwQuerySystemInformation fnZwQuerySystemInformation = NULL; @@ -2188,49 +2182,42 @@ static PZwQuerySystemInformation fnZwQuerySystemInformation = NULL; static BOOL CreateProcessSnapshot(VOID** snapshotContext) { HMODULE hModule = NULL; - ULONG cbBuffer = 1024 * 1024; // 1Mb - default process information buffer size (that's enough in most cases for high-loaded systems) - LPVOID pBuffer = NULL; - NTSTATUS status = 0; + ULONG cbBuffer = 1024 * 1024; // 1Mb - default process information buffer size (that's enough in most cases for high-loaded systems) + LPVOID pBuffer = NULL; + NTSTATUS status = 0; - if (!fnZwQuerySystemInformation) - { + if (!fnZwQuerySystemInformation) { hModule = GetModuleHandle(TEXT("ntdll.dll")); - if (!hModule) - { - return FALSE; - } - fnZwQuerySystemInformation = (PZwQuerySystemInformation)GetProcAddress(hModule, "ZwQuerySystemInformation"); - if (!fnZwQuerySystemInformation) - { - return FALSE; - } - } - - do - { - pBuffer = DetourCreateObjectArray(cbBuffer); - if (pBuffer == NULL) - { - return FALSE; - } - - status = fnZwQuerySystemInformation(SystemProcessInformation, pBuffer, cbBuffer, NULL); - - if (status == STATUS_INFO_LENGTH_MISMATCH) - { - DetourDestroyObjectArray((LPBYTE)pBuffer); - cbBuffer *= 2; - } - else if (status < 0) - { - DetourDestroyObjectArray((LPBYTE)pBuffer); - return FALSE; - } - } while (status == STATUS_INFO_LENGTH_MISMATCH); - - *snapshotContext = pBuffer; - - return TRUE; + if (!hModule) { + return FALSE; + } + fnZwQuerySystemInformation = (PZwQuerySystemInformation)GetProcAddress(hModule, "ZwQuerySystemInformation"); + if (!fnZwQuerySystemInformation) { + return FALSE; + } + } + + do { + pBuffer = DetourCreateObjectArray(cbBuffer); + if (pBuffer == NULL) { + return FALSE; + } + + status = fnZwQuerySystemInformation(SystemProcessInformation, pBuffer, cbBuffer, NULL); + + if (status == STATUS_INFO_LENGTH_MISMATCH) { + DetourDestroyObjectArray((LPBYTE)pBuffer); + cbBuffer *= 2; + } + else if (status < 0) { + DetourDestroyObjectArray((LPBYTE)pBuffer); + return FALSE; + } + } while (status == STATUS_INFO_LENGTH_MISMATCH); + + *snapshotContext = pBuffer; + + return TRUE; } //========================================================================= @@ -2240,26 +2227,22 @@ static BOOL CreateProcessSnapshot(VOID** snapshotContext) //========================================================================= static PSYSTEM_PROCESS_INFORMATION FindProcess(VOID* snapshotContext, SIZE_T processId) { - PSYSTEM_PROCESS_INFORMATION currentProcess = (PSYSTEM_PROCESS_INFORMATION)snapshotContext; + PSYSTEM_PROCESS_INFORMATION currentProcess = (PSYSTEM_PROCESS_INFORMATION)snapshotContext; - while (currentProcess != NULL) - { - if (currentProcess->uUniqueProcessId == (HANDLE)processId) - { - break; - } + while (currentProcess != NULL) { + if (currentProcess->uUniqueProcessId == (HANDLE)processId) { + break; + } - if (currentProcess->uNext == 0) - { - currentProcess = NULL; - } - else - { - currentProcess = (PSYSTEM_PROCESS_INFORMATION)(((LPBYTE)currentProcess) + currentProcess->uNext); - } - } + if (currentProcess->uNext == 0) { + currentProcess = NULL; + } + else { + currentProcess = (PSYSTEM_PROCESS_INFORMATION)(((LPBYTE)currentProcess) + currentProcess->uNext); + } + } - return currentProcess; + return currentProcess; } //========================================================================= @@ -2270,18 +2253,17 @@ static PSYSTEM_PROCESS_INFORMATION FindProcess(VOID* snapshotContext, SIZE_T pro //========================================================================= static BOOL GetCurrentProcessSnapshot(PVOID* snapshot, PSYSTEM_PROCESS_INFORMATION* procInfo) { - // get a view of the threads in the system + // get a view of the threads in the system - if (!CreateProcessSnapshot(snapshot)) - { - DETOUR_TRACE(("detours: can't get process snapshot!")); - return FALSE; - } + if (!CreateProcessSnapshot(snapshot)) { + DETOUR_TRACE(("detours: can't get process snapshot!")); + return FALSE; + } - DWORD pid = GetCurrentProcessId(); + DWORD pid = GetCurrentProcessId(); - *procInfo = FindProcess(*snapshot, pid); - return TRUE; + *procInfo = FindProcess(*snapshot, pid); + return TRUE; } //========================================================================= @@ -2291,50 +2273,45 @@ static BOOL GetCurrentProcessSnapshot(PVOID* snapshot, PSYSTEM_PROCESS_INFORMATI //========================================================================= static VOID CloseProcessSnapshot(VOID* snapshotContext) { - DetourDestroyObjectArray((LPBYTE)snapshotContext); + DetourDestroyObjectArray((LPBYTE)snapshotContext); } BOOL WINAPI DetourUpdateAllOtherThreads() { - LONG bResult = FALSE; - - VOID* procEnumerationCtx = NULL; - PSYSTEM_PROCESS_INFORMATION procInfo = NULL; - - if (GetCurrentProcessSnapshot(&procEnumerationCtx, &procInfo)) - { - DWORD tid = GetCurrentThreadId(); - // go through every thread - for (ULONG threadIdx = 0; threadIdx < procInfo->uThreadCount; threadIdx++) - { - DWORD threadId = (DWORD)(DWORD_PTR)procInfo->Threads[threadIdx].ClientId.UniqueThread; - - if (threadId != tid) - { - HANDLE hThread = OpenThread(THREAD_SUSPEND_RESUME | THREAD_GET_CONTEXT | THREAD_QUERY_INFORMATION | THREAD_SET_CONTEXT, FALSE, threadId); - if (hThread) - { - LONG error = DetourUpdateThreadEx(hThread, TRUE); - assert(error == NO_ERROR); - if (error) - { - DETOUR_TRACE(("DetourUpdateThreadEx failed, error=%d\n", error)); - //Reset s_nPendingError so that subsequent threads can continue to try to call the DetourUpdateThreadEx function - assert(error == s_nPendingError); - //When the console program is terminated, s_nPendingError == ERROR_ACCESS_DENIED may be caused when the SuspendThread is called in DetourUpdateThreadEx - assert(s_nPendingError == ERROR_ACCESS_DENIED); - s_nPendingError = NO_ERROR; - } - } - } - } - - CloseProcessSnapshot(procEnumerationCtx); - - bResult = TRUE; - } - - return bResult; + LONG bResult = FALSE; + + VOID* procEnumerationCtx = NULL; + PSYSTEM_PROCESS_INFORMATION procInfo = NULL; + + if (GetCurrentProcessSnapshot(&procEnumerationCtx, &procInfo)) { + DWORD tid = GetCurrentThreadId(); + // go through every thread + for (ULONG threadIdx = 0; threadIdx < procInfo->uThreadCount; threadIdx++) { + DWORD threadId = (DWORD)(DWORD_PTR)procInfo->Threads[threadIdx].ClientId.UniqueThread; + + if (threadId != tid) { + HANDLE hThread = OpenThread(THREAD_SUSPEND_RESUME | THREAD_GET_CONTEXT | THREAD_QUERY_INFORMATION | THREAD_SET_CONTEXT, FALSE, threadId); + if (hThread) { + LONG error = DetourUpdateThreadEx(hThread, TRUE); + assert(error == NO_ERROR); + if (error) { + DETOUR_TRACE(("DetourUpdateThreadEx failed, error=%d\n", error)); + //Reset s_nPendingError so that subsequent threads can continue to try to call the DetourUpdateThreadEx function + assert(error == s_nPendingError); + //When the console program is terminated, s_nPendingError == ERROR_ACCESS_DENIED may be caused when the SuspendThread is called in DetourUpdateThreadEx + assert(s_nPendingError == ERROR_ACCESS_DENIED); + s_nPendingError = NO_ERROR; + } + } + } + } + + CloseProcessSnapshot(procEnumerationCtx); + + bResult = TRUE; + } + + return bResult; } ///////////////////////////////////////////////////////////// Transacted APIs. diff --git a/src/detours.h b/src/detours.h index 6fc1b9e6..050c756c 100644 --- a/src/detours.h +++ b/src/detours.h @@ -1136,100 +1136,91 @@ BOOL WINAPI DetourVirtualProtectSameExecute(_In_ PVOID pAddress, template T* DetourCreateObject() { - T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T)); - assert(p); - if (!p) - { - return p; - } + T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T)); + assert(p); + if (!p) { + return p; + } - ::new(p) T; + ::new(p) T; - return p; + return p; } template T* DetourCreateObject(P1 p1) { - T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T)); - assert(p); - if (!p) - { - return p; - } + T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T)); + assert(p); + if (!p) { + return p; + } - ::new(p) T(p1); + ::new(p) T(p1); - return p; + return p; } template T* DetourCreateObject(P1 p1, P2 p2) { - T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T)); - assert(p); - if (!p) - { - return p; - } + T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T)); + assert(p); + if (!p) { + return p; + } - ::new(p) T(p1, p2); + ::new(p) T(p1, p2); - return p; + return p; } template void DetourDestroyObject(T* p) { - size_t MemSize = HeapSize(DetourGetHeap(), 0, p); - assert(MemSize == sizeof(T)); - if (MemSize != sizeof(T)) - { - return; - } + size_t MemSize = HeapSize(DetourGetHeap(), 0, p); + assert(MemSize == sizeof(T)); + if (MemSize != sizeof(T)) { + return; + } - p->~T(); + p->~T(); - HeapFree(DetourGetHeap(), 0, p); - p = NULL; + HeapFree(DetourGetHeap(), 0, p); + p = NULL; } template T* DetourCreateObjectArray(size_t _Size) { - T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T) * _Size); - assert(p); - if (!p) - { - return p; - } - - for (size_t i = 0; i < _Size; i++) - { - ::new(&p[i]) T; - } - - return p; + T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T) * _Size); + assert(p); + if (!p) { + return p; + } + + for (size_t i = 0; i < _Size; i++) { + ::new(&p[i]) T; + } + + return p; } template void DetourDestroyObjectArray(T* p) { - size_t MemSize = HeapSize(DetourGetHeap(), 0, p); - assert(MemSize > 0); - if (MemSize == 0) - { - return; - } - size_t _Size = MemSize / sizeof(T); - assert(_Size > 0); - if (_Size == 0) - { - return; - } - - for (size_t i = 0; i < _Size; i++) - { - (&p[i])->~T(); - } - - HeapFree(DetourGetHeap(), 0, (LPVOID)p); - p = NULL; + size_t MemSize = HeapSize(DetourGetHeap(), 0, p); + assert(MemSize > 0); + if (MemSize == 0) { + return; + } + size_t _Size = MemSize / sizeof(T); + assert(_Size > 0); + if (_Size == 0) { + return; + } + + for (size_t i = 0; i < _Size; i++) { + (&p[i])->~T(); + } + + HeapFree(DetourGetHeap(), 0, (LPVOID)p); + p = NULL; } ////////////////////////////////////////////////////////////////////////////// From 08e1e1c3cbe4ebdf6b22bbaa68860e32fb0a82e5 Mon Sep 17 00:00:00 2001 From: sonyps5201314 Date: Mon, 7 Sep 2020 23:01:21 +0800 Subject: [PATCH 03/14] The code format is adapted to the Detours code style --- src/detours.cpp | 6 +++--- src/detours.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/detours.cpp b/src/detours.cpp index 82cca96b..a7d9e56a 100644 --- a/src/detours.cpp +++ b/src/detours.cpp @@ -2041,7 +2041,7 @@ LONG WINAPI DetourUpdateThreadEx(_In_ HANDLE hThread, _In_opt_ BOOL fCloseThread return NO_ERROR; } -//Code modified from https://github.com/apriorit/mhook/blob/master/mhook-lib/mhook.c +// Code modified from https://github.com/apriorit/mhook/blob/master/mhook-lib/mhook.c //========================================================================= // ntdll definitions @@ -2296,9 +2296,9 @@ BOOL WINAPI DetourUpdateAllOtherThreads() assert(error == NO_ERROR); if (error) { DETOUR_TRACE(("DetourUpdateThreadEx failed, error=%d\n", error)); - //Reset s_nPendingError so that subsequent threads can continue to try to call the DetourUpdateThreadEx function + // Reset s_nPendingError so that subsequent threads can continue to try to call the DetourUpdateThreadEx function assert(error == s_nPendingError); - //When the console program is terminated, s_nPendingError == ERROR_ACCESS_DENIED may be caused when the SuspendThread is called in DetourUpdateThreadEx + // When the console program is terminated, s_nPendingError == ERROR_ACCESS_DENIED may be caused when the SuspendThread is called in DetourUpdateThreadEx assert(s_nPendingError == ERROR_ACCESS_DENIED); s_nPendingError = NO_ERROR; } diff --git a/src/detours.h b/src/detours.h index 050c756c..f6e9a9d0 100644 --- a/src/detours.h +++ b/src/detours.h @@ -557,7 +557,7 @@ LONG WINAPI DetourTransactionAbort(VOID); LONG WINAPI DetourTransactionCommit(VOID); LONG WINAPI DetourTransactionCommitEx(_Out_opt_ PVOID **pppFailedPointer); -//DetourUpdateThread is no longer recommended to be called by users, please use DetourUpdateAllOtherThreads for safer HOOK +// DetourUpdateThread is no longer recommended to be called by users, please use DetourUpdateAllOtherThreads for safer HOOK LONG WINAPI DetourUpdateThread(_In_ HANDLE hThread); LONG WINAPI DetourUpdateThreadEx(_In_ HANDLE hThread, _In_opt_ BOOL fCloseThreadHandleOnDestroyDetourThreadObject /*= TRUE*/); BOOL WINAPI DetourUpdateAllOtherThreads(); From cfa9dfc599e13793ef65616e6fe2778cdebe0a72 Mon Sep 17 00:00:00 2001 From: sonyps5201314 Date: Tue, 15 Sep 2020 18:52:22 +0800 Subject: [PATCH 04/14] Added notes on usage precautions to DetourUpdateAllOtherThreads --- src/detours.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/detours.h b/src/detours.h index f6e9a9d0..09e89296 100644 --- a/src/detours.h +++ b/src/detours.h @@ -559,6 +559,11 @@ LONG WINAPI DetourTransactionCommitEx(_Out_opt_ PVOID **pppFailedPointer); // DetourUpdateThread is no longer recommended to be called by users, please use DetourUpdateAllOtherThreads for safer HOOK LONG WINAPI DetourUpdateThread(_In_ HANDLE hThread); + +// After calling DetourUpdateAllOtherThreads, you should call DetourTransactionCommit(Ex) or DetourTransactionAbort as soon as possible. +// In addition to the DetourAttach(Ex)\DetourDetach(Ex) call, other user operations should not be included between them, +// because other user operations may cause CRT lock competition, and at this time CRT lock may be owned by other threads. +// Other threads have been suspended at this time, so that may cause deadlock problems LONG WINAPI DetourUpdateThreadEx(_In_ HANDLE hThread, _In_opt_ BOOL fCloseThreadHandleOnDestroyDetourThreadObject /*= TRUE*/); BOOL WINAPI DetourUpdateAllOtherThreads(); From b536f107ae3e117130e8e5d6ac56f442c224a3a2 Mon Sep 17 00:00:00 2001 From: sonyps5201314 Date: Wed, 21 Oct 2020 01:50:30 +0800 Subject: [PATCH 05/14] sync from mhook: Add comment and assert for FindProcess (fix #26) --- src/detours.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/detours.cpp b/src/detours.cpp index a7d9e56a..e362c8a6 100644 --- a/src/detours.cpp +++ b/src/detours.cpp @@ -2260,9 +2260,10 @@ static BOOL GetCurrentProcessSnapshot(PVOID* snapshot, PSYSTEM_PROCESS_INFORMATI return FALSE; } - DWORD pid = GetCurrentProcessId(); + // this never returns NULL as the current process id is always present in the processes snapshot + *procInfo = FindProcess(*snapshot, GetCurrentProcessId()); + assert(procInfo); - *procInfo = FindProcess(*snapshot, pid); return TRUE; } From cde95a84218cfb38b90c24112e6f4ff84fc9f3fa Mon Sep 17 00:00:00 2001 From: sonyps5201314 Date: Wed, 21 Oct 2020 03:48:28 +0800 Subject: [PATCH 06/14] Use virtual memory to replace heap memory for internal memory management operations in Detours, because heap memory API functions may be HOOKed. For example, in AcLayers.dll, there may be lock competition between HOOK heap functions. This is only a temporary circumvention solution, the final solution It should also be ensured that the virtual memory API is not locked and can be called. --- samples/common.mak | 2 +- src/detours.cpp | 37 ------------- src/detours.h | 133 +++++++++++++++++++++++++++++++++++---------- 3 files changed, 104 insertions(+), 68 deletions(-) diff --git a/samples/common.mak b/samples/common.mak index 59b7ac53..aa63a156 100644 --- a/samples/common.mak +++ b/samples/common.mak @@ -23,7 +23,7 @@ CLIB=/MT !ENDIF AFLAGS=/nologo /Zi /c /Fl -CFLAGS=/nologo /Zi $(CLIB) /Gm- /W4 /WX /we4777 /we4800 /Od /D__AUTO_CREATE_DETOUR_HEAP__ +CFLAGS=/nologo /Zi $(CLIB) /Gm- /W4 /WX /we4777 /we4800 /Od !IF $(DETOURS_SOURCE_BROWSING)==1 CFLAGS=$(CFLAGS) /FR diff --git a/src/detours.cpp b/src/detours.cpp index e362c8a6..cb9a73ad 100644 --- a/src/detours.cpp +++ b/src/detours.cpp @@ -1543,43 +1543,6 @@ static PVOID * s_ppPendingError = NULL; static DetourThread * s_pPendingThreads = NULL; static DetourOperation * s_pPendingOperations = NULL; -////////////////////////////////////////////////////////////////////////////// -// -void WINAPI DetourDestroyHeap() -{ - if (s_hHeap) { - HeapDestroy(s_hHeap); - s_hHeap = NULL; - } -} - -static void DetourDestroyHeap__for__atexit() -{ - DetourDestroyHeap(); -} - -BOOL WINAPI DetourCreateHeap(BOOL fAutoDestroy) -{ - BOOL bResult = FALSE; - if (s_hHeap == NULL) { - s_hHeap = HeapCreate(0, 0, 0); - assert(s_hHeap); - if (s_hHeap != NULL) { - if (fAutoDestroy) { - atexit(DetourDestroyHeap__for__atexit); - } - bResult = TRUE; - } - } - return bResult; -} - -HANDLE WINAPI DetourGetHeap() -{ - assert(s_hHeap); - return s_hHeap; -} - ////////////////////////////////////////////////////////////////////////////// // PVOID WINAPI DetourCodeFromPointer(_In_ PVOID pPointer, diff --git a/src/detours.h b/src/detours.h index 09e89296..1df7c1f3 100644 --- a/src/detours.h +++ b/src/detours.h @@ -541,14 +541,6 @@ typedef BOOL (CALLBACK *PF_DETOUR_IMPORT_FUNC_CALLBACK_EX)(_In_opt_ PVOID pConte typedef VOID * PDETOUR_BINARY; typedef VOID * PDETOUR_LOADED_BINARY; -//////////////////////////////////////////////////////////// Memory management APIs. -BOOL WINAPI DetourCreateHeap(BOOL fAutoDestroy); -HANDLE WINAPI DetourGetHeap(); -void WINAPI DetourDestroyHeap(); -#ifdef __AUTO_CREATE_DETOUR_HEAP__ -static BOOL s_fAlreadyCreatedDetourHeap = DetourCreateHeap(TRUE); -#endif - //////////////////////////////////////////////////////////// Transaction APIs. // LONG WINAPI DetourTransactionBegin(VOID); @@ -1138,15 +1130,83 @@ BOOL WINAPI DetourVirtualProtectSameExecute(_In_ PVOID pAddress, #endif // __cplusplus //////////////////////////////////////////////////////////// Memory management APIs. +struct DetourMemory +{ + DWORD dwHead; + size_t nObjectSize; + //T object + //DWORD dwTail; +public: + DetourMemory(size_t nObjectSize, BOOL bOnlyForGetStructSize) + { + dwHead = 0x12345678; + this->nObjectSize = nObjectSize; + if (!bOnlyForGetStructSize) { + *TailPtr() = 0x9ABCDEF0; + } + } +public: + size_t GetObjectSize() const + { + return nObjectSize; + } + size_t GetStructSize() const + { + size_t nStructSize = sizeof(DetourMemory) + nObjectSize + sizeof(DWORD); + return nStructSize; + } + PBYTE ObjectPtr() + { + PBYTE p = ((PBYTE)this) + sizeof(DetourMemory); + return p; + } + PDWORD TailPtr() + { + PDWORD pdwTail = (PDWORD)(ObjectPtr() + nObjectSize); + return pdwTail; + } + BOOL Validate() + { + BOOL bIsValid = dwHead == 0x12345678; + assert(bIsValid); + if (bIsValid) { + bIsValid = *TailPtr() == 0x9ABCDEF0; + assert(bIsValid); + } + return bIsValid; + } +public: + static DetourMemory* FromObjectPtr(PBYTE p) + { + PBYTE pm = p - sizeof(DetourMemory); + return (DetourMemory*)pm; + } +}; + +inline PVOID DetourMemAlloc(size_t size) +{ + PVOID memblock = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE); + assert(memblock); + return memblock; +} +inline VOID DetourMemFree(PVOID memblock) +{ + BOOL bResult = VirtualFree(memblock, 0, MEM_RELEASE); + assert(bResult); +} + template T* DetourCreateObject() { - T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T)); - assert(p); - if (!p) { - return p; + DetourMemory m(sizeof(T), TRUE); + DetourMemory* pm = (DetourMemory*)DetourMemAlloc(m.GetStructSize()); + assert(pm); + if (!pm) { + return NULL; } + ::new(pm) DetourMemory(m.GetObjectSize(), FALSE); + T* p = (T*)pm->ObjectPtr(); ::new(p) T; return p; @@ -1154,12 +1214,15 @@ T* DetourCreateObject() template T* DetourCreateObject(P1 p1) { - T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T)); - assert(p); - if (!p) { - return p; + DetourMemory m(sizeof(T), TRUE); + DetourMemory* pm = (DetourMemory*)DetourMemAlloc(m.GetStructSize()); + assert(pm); + if (!pm) { + return NULL; } + ::new(pm) DetourMemory(m.GetObjectSize(), FALSE); + T* p = (T*)pm->ObjectPtr(); ::new(p) T(p1); return p; @@ -1167,12 +1230,15 @@ T* DetourCreateObject(P1 p1) template T* DetourCreateObject(P1 p1, P2 p2) { - T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T)); - assert(p); - if (!p) { - return p; + DetourMemory m(sizeof(T), TRUE); + DetourMemory* pm = (DetourMemory*)DetourMemAlloc(m.GetStructSize()); + assert(pm); + if (!pm) { + return NULL; } + ::new(pm) DetourMemory(m.GetObjectSize(), FALSE); + T* p = (T*)pm->ObjectPtr(); ::new(p) T(p1, p2); return p; @@ -1180,7 +1246,8 @@ T* DetourCreateObject(P1 p1, P2 p2) template void DetourDestroyObject(T* p) { - size_t MemSize = HeapSize(DetourGetHeap(), 0, p); + DetourMemory* pm = DetourMemory::FromObjectPtr((PBYTE)p); + size_t MemSize = pm->GetObjectSize(); assert(MemSize == sizeof(T)); if (MemSize != sizeof(T)) { return; @@ -1188,18 +1255,22 @@ void DetourDestroyObject(T* p) p->~T(); - HeapFree(DetourGetHeap(), 0, p); - p = NULL; + pm->Validate(); + DetourMemFree((PVOID)pm); + pm = NULL; } template T* DetourCreateObjectArray(size_t _Size) { - T* p = (T*)HeapAlloc(DetourGetHeap(), 0, sizeof(T) * _Size); - assert(p); - if (!p) { - return p; + DetourMemory m(sizeof(T) * _Size, TRUE); + DetourMemory* pm = (DetourMemory*)DetourMemAlloc(m.GetStructSize()); + assert(pm); + if (!pm) { + return NULL; } + ::new(pm) DetourMemory(m.GetObjectSize(), FALSE); + T* p = (T*)pm->ObjectPtr(); for (size_t i = 0; i < _Size; i++) { ::new(&p[i]) T; } @@ -1209,7 +1280,8 @@ T* DetourCreateObjectArray(size_t _Size) template void DetourDestroyObjectArray(T* p) { - size_t MemSize = HeapSize(DetourGetHeap(), 0, p); + DetourMemory* pm = DetourMemory::FromObjectPtr((PBYTE)p); + size_t MemSize = pm->GetObjectSize(); assert(MemSize > 0); if (MemSize == 0) { return; @@ -1224,8 +1296,9 @@ void DetourDestroyObjectArray(T* p) (&p[i])->~T(); } - HeapFree(DetourGetHeap(), 0, (LPVOID)p); - p = NULL; + pm->Validate(); + DetourMemFree((PVOID)pm); + pm = NULL; } ////////////////////////////////////////////////////////////////////////////// From 1fe5b0e6c7606c90d8c734aba388f4177bcdb2a7 Mon Sep 17 00:00:00 2001 From: sonyps5201314 Date: Wed, 21 Oct 2020 11:52:19 +0800 Subject: [PATCH 07/14] Refine the code --- src/creatwth.cpp | 4 +- src/detours.cpp | 16 ++--- src/detours.h | 179 +++++++++++++++++++++++++---------------------- 3 files changed, 106 insertions(+), 93 deletions(-) diff --git a/src/creatwth.cpp b/src/creatwth.cpp index aa209db3..b64a214f 100644 --- a/src/creatwth.cpp +++ b/src/creatwth.cpp @@ -1145,7 +1145,7 @@ BOOL WINAPI AllocExeHelper(_Out_ PDETOUR_EXE_HELPER *pHelper, Cleanup: if (Helper != NULL) { - DetourDestroyObjectArray((PBYTE)Helper); + DetourDestroyObjectArray((PBYTE&)Helper); Helper = NULL; } return Result; @@ -1155,7 +1155,7 @@ static VOID WINAPI FreeExeHelper(PDETOUR_EXE_HELPER *pHelper) { if (*pHelper != NULL) { - DetourDestroyObjectArray((PBYTE)*pHelper); + DetourDestroyObjectArray((PBYTE&)*pHelper); *pHelper = NULL; } } diff --git a/src/detours.cpp b/src/detours.cpp index cb9a73ad..e7fe3a64 100644 --- a/src/detours.cpp +++ b/src/detours.cpp @@ -2142,11 +2142,11 @@ static PZwQuerySystemInformation fnZwQuerySystemInformation = NULL; // // Get snapshot of the processes started in the system //========================================================================= -static BOOL CreateProcessSnapshot(VOID** snapshotContext) +static BOOL CreateProcessSnapshot(PBYTE* snapshotContext) { HMODULE hModule = NULL; ULONG cbBuffer = 1024 * 1024; // 1Mb - default process information buffer size (that's enough in most cases for high-loaded systems) - LPVOID pBuffer = NULL; + PBYTE pBuffer = NULL; NTSTATUS status = 0; if (!fnZwQuerySystemInformation) { @@ -2169,11 +2169,11 @@ static BOOL CreateProcessSnapshot(VOID** snapshotContext) status = fnZwQuerySystemInformation(SystemProcessInformation, pBuffer, cbBuffer, NULL); if (status == STATUS_INFO_LENGTH_MISMATCH) { - DetourDestroyObjectArray((LPBYTE)pBuffer); + DetourDestroyObjectArray(pBuffer); cbBuffer *= 2; } else if (status < 0) { - DetourDestroyObjectArray((LPBYTE)pBuffer); + DetourDestroyObjectArray(pBuffer); return FALSE; } } while (status == STATUS_INFO_LENGTH_MISMATCH); @@ -2214,7 +2214,7 @@ static PSYSTEM_PROCESS_INFORMATION FindProcess(VOID* snapshotContext, SIZE_T pro // Get current process snapshot and process info // //========================================================================= -static BOOL GetCurrentProcessSnapshot(PVOID* snapshot, PSYSTEM_PROCESS_INFORMATION* procInfo) +static BOOL GetCurrentProcessSnapshot(PBYTE* snapshot, PSYSTEM_PROCESS_INFORMATION* procInfo) { // get a view of the threads in the system @@ -2235,16 +2235,16 @@ static BOOL GetCurrentProcessSnapshot(PVOID* snapshot, PSYSTEM_PROCESS_INFORMATI // // Free memory allocated for processes snapshot //========================================================================= -static VOID CloseProcessSnapshot(VOID* snapshotContext) +static VOID CloseProcessSnapshot(PBYTE& snapshotContext) { - DetourDestroyObjectArray((LPBYTE)snapshotContext); + DetourDestroyObjectArray(snapshotContext); } BOOL WINAPI DetourUpdateAllOtherThreads() { LONG bResult = FALSE; - VOID* procEnumerationCtx = NULL; + PBYTE procEnumerationCtx = NULL; PSYSTEM_PROCESS_INFORMATION procInfo = NULL; if (GetCurrentProcessSnapshot(&procEnumerationCtx, &procInfo)) { diff --git a/src/detours.h b/src/detours.h index 1df7c1f3..59687786 100644 --- a/src/detours.h +++ b/src/detours.h @@ -1136,7 +1136,7 @@ struct DetourMemory size_t nObjectSize; //T object //DWORD dwTail; -public: +private: DetourMemory(size_t nObjectSize, BOOL bOnlyForGetStructSize) { dwHead = 0x12345678; @@ -1160,6 +1160,7 @@ struct DetourMemory PBYTE p = ((PBYTE)this) + sizeof(DetourMemory); return p; } +private: PDWORD TailPtr() { PDWORD pdwTail = (PDWORD)(ObjectPtr() + nObjectSize); @@ -1175,130 +1176,142 @@ struct DetourMemory } return bIsValid; } +private: + static inline PVOID DetourMemAlloc(size_t size) + { + PVOID memblock = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE); + assert(memblock); + return memblock; + } + static inline BOOL DetourMemFree(PVOID memblock) + { + BOOL bResult = VirtualFree(memblock, 0, MEM_RELEASE); + assert(bResult); + return bResult; + } +public: + static DetourMemory* Create(size_t nObjectSize) + { + DetourMemory m(nObjectSize, TRUE); + DetourMemory* pm = (DetourMemory*)DetourMemAlloc(m.GetStructSize()); + assert(pm); + if (pm) { + ::new(pm) DetourMemory(m.GetObjectSize(), FALSE); + } + return pm; + } + static BOOL Destroy(DetourMemory*& pm, BOOL bValidate) + { + BOOL bResult = FALSE; + if (pm) { + bResult = bValidate ? pm->Validate() : TRUE; + if (bResult) { + bResult = DetourMemFree((PVOID)pm); + if (bResult) { + pm = NULL; + } + } + } + return bResult; + } public: static DetourMemory* FromObjectPtr(PBYTE p) { - PBYTE pm = p - sizeof(DetourMemory); - return (DetourMemory*)pm; + DetourMemory* pm = NULL; + if (p) { + pm = (DetourMemory*)(p - sizeof(DetourMemory)); + if (!pm->Validate()) { + pm = NULL; + } + } + return pm; } }; -inline PVOID DetourMemAlloc(size_t size) -{ - PVOID memblock = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE); - assert(memblock); - return memblock; -} -inline VOID DetourMemFree(PVOID memblock) -{ - BOOL bResult = VirtualFree(memblock, 0, MEM_RELEASE); - assert(bResult); -} - template T* DetourCreateObject() { - DetourMemory m(sizeof(T), TRUE); - DetourMemory* pm = (DetourMemory*)DetourMemAlloc(m.GetStructSize()); - assert(pm); - if (!pm) { - return NULL; + DetourMemory* pm = DetourMemory::Create(sizeof(T)); + T* p = NULL; + if (pm) { + p = (T*)pm->ObjectPtr(); + ::new(p) T; } - ::new(pm) DetourMemory(m.GetObjectSize(), FALSE); - - T* p = (T*)pm->ObjectPtr(); - ::new(p) T; - return p; } template T* DetourCreateObject(P1 p1) { - DetourMemory m(sizeof(T), TRUE); - DetourMemory* pm = (DetourMemory*)DetourMemAlloc(m.GetStructSize()); - assert(pm); - if (!pm) { - return NULL; + DetourMemory* pm = DetourMemory::Create(sizeof(T)); + T* p = NULL; + if (pm) { + p = (T*)pm->ObjectPtr(); + ::new(p) T(p1); } - ::new(pm) DetourMemory(m.GetObjectSize(), FALSE); - - T* p = (T*)pm->ObjectPtr(); - ::new(p) T(p1); - return p; } template T* DetourCreateObject(P1 p1, P2 p2) { - DetourMemory m(sizeof(T), TRUE); - DetourMemory* pm = (DetourMemory*)DetourMemAlloc(m.GetStructSize()); - assert(pm); - if (!pm) { - return NULL; + DetourMemory* pm = DetourMemory::Create(sizeof(T)); + T* p = NULL; + if (pm) { + p = (T*)pm->ObjectPtr(); + ::new(p) T(p1, p2); } - ::new(pm) DetourMemory(m.GetObjectSize(), FALSE); - - T* p = (T*)pm->ObjectPtr(); - ::new(p) T(p1, p2); - return p; } template -void DetourDestroyObject(T* p) +void DetourDestroyObject(T*& p) { DetourMemory* pm = DetourMemory::FromObjectPtr((PBYTE)p); - size_t MemSize = pm->GetObjectSize(); - assert(MemSize == sizeof(T)); - if (MemSize != sizeof(T)) { - return; - } + if (pm) { + size_t MemSize = pm->GetObjectSize(); + assert(MemSize == sizeof(T)); + if (MemSize != sizeof(T)) { + return; + } - p->~T(); + p->~T(); + p = NULL; - pm->Validate(); - DetourMemFree((PVOID)pm); - pm = NULL; + DetourMemory::Destroy(pm, FALSE); + } } template T* DetourCreateObjectArray(size_t _Size) { - DetourMemory m(sizeof(T) * _Size, TRUE); - DetourMemory* pm = (DetourMemory*)DetourMemAlloc(m.GetStructSize()); - assert(pm); - if (!pm) { - return NULL; - } - ::new(pm) DetourMemory(m.GetObjectSize(), FALSE); - - T* p = (T*)pm->ObjectPtr(); - for (size_t i = 0; i < _Size; i++) { - ::new(&p[i]) T; + DetourMemory* pm = DetourMemory::Create(sizeof(T) * _Size); + T* p = NULL; + if (pm) { + p = (T*)pm->ObjectPtr(); + for (size_t i = 0; i < _Size; i++) { + ::new(&p[i]) T; + } } return p; } template -void DetourDestroyObjectArray(T* p) +void DetourDestroyObjectArray(T*& p) { DetourMemory* pm = DetourMemory::FromObjectPtr((PBYTE)p); - size_t MemSize = pm->GetObjectSize(); - assert(MemSize > 0); - if (MemSize == 0) { - return; - } - size_t _Size = MemSize / sizeof(T); - assert(_Size > 0); - if (_Size == 0) { - return; - } + if (pm) { + size_t MemSize = pm->GetObjectSize(); + assert(MemSize % sizeof(T) == 0); + if (MemSize % sizeof(T) != 0) { + return; + } - for (size_t i = 0; i < _Size; i++) { - (&p[i])->~T(); - } + size_t _Size = MemSize / sizeof(T); + //_Size equal to 0 is allowed + for (size_t i = 0; i < _Size; i++) { + (&p[i])->~T(); + } + p = NULL; - pm->Validate(); - DetourMemFree((PVOID)pm); - pm = NULL; + DetourMemory::Destroy(pm, FALSE); + } } ////////////////////////////////////////////////////////////////////////////// From 961977ae9609440fdcda17bc573e55217e37b2b6 Mon Sep 17 00:00:00 2001 From: sonyps5201314 Date: Sun, 25 Oct 2020 10:39:26 +0800 Subject: [PATCH 08/14] make Memory-management-APIs can avoid Use-after-free, Double-free and Over-bound-access memory errors. --- src/detours.cpp | 24 +++- src/detours.h | 327 ++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 279 insertions(+), 72 deletions(-) diff --git a/src/detours.cpp b/src/detours.cpp index e7fe3a64..8caa35c6 100644 --- a/src/detours.cpp +++ b/src/detours.cpp @@ -18,6 +18,22 @@ #define NOTHROW +////////////////////////////////////////////////////////////////////////////// +// + +extern "C" void __cdecl _assert(const char* expr, const char* filename, unsigned lineno); +void DetourAssert(const char* expr, const char* filename, unsigned lineno) +{ + DWORD dwLastError = GetLastError(); + SetLastError(dwLastError); + int last_error_mode = _set_error_mode(_OUT_TO_MSGBOX); + _assert(expr, filename, lineno); + if (last_error_mode != _OUT_TO_MSGBOX) { + _set_error_mode(last_error_mode); + } + SetLastError(dwLastError); +} + ////////////////////////////////////////////////////////////////////////////// // struct _DETOUR_ALIGN @@ -2225,7 +2241,7 @@ static BOOL GetCurrentProcessSnapshot(PBYTE* snapshot, PSYSTEM_PROCESS_INFORMATI // this never returns NULL as the current process id is always present in the processes snapshot *procInfo = FindProcess(*snapshot, GetCurrentProcessId()); - assert(procInfo); + DETOUR_ASSERT(procInfo); return TRUE; } @@ -2257,13 +2273,13 @@ BOOL WINAPI DetourUpdateAllOtherThreads() HANDLE hThread = OpenThread(THREAD_SUSPEND_RESUME | THREAD_GET_CONTEXT | THREAD_QUERY_INFORMATION | THREAD_SET_CONTEXT, FALSE, threadId); if (hThread) { LONG error = DetourUpdateThreadEx(hThread, TRUE); - assert(error == NO_ERROR); + DETOUR_ASSERT(error == NO_ERROR); if (error) { DETOUR_TRACE(("DetourUpdateThreadEx failed, error=%d\n", error)); // Reset s_nPendingError so that subsequent threads can continue to try to call the DetourUpdateThreadEx function - assert(error == s_nPendingError); + DETOUR_ASSERT(error == s_nPendingError); // When the console program is terminated, s_nPendingError == ERROR_ACCESS_DENIED may be caused when the SuspendThread is called in DetourUpdateThreadEx - assert(s_nPendingError == ERROR_ACCESS_DENIED); + DETOUR_ASSERT(s_nPendingError == ERROR_ACCESS_DENIED); s_nPendingError = NO_ERROR; } } diff --git a/src/detours.h b/src/detours.h index 59687786..76be688a 100644 --- a/src/detours.h +++ b/src/detours.h @@ -48,7 +48,7 @@ #pragma warning(pop) #endif #include -#include +#include // From winerror.h, as this error isn't found in some SDKs: // @@ -551,10 +551,10 @@ LONG WINAPI DetourTransactionCommitEx(_Out_opt_ PVOID **pppFailedPointer); // DetourUpdateThread is no longer recommended to be called by users, please use DetourUpdateAllOtherThreads for safer HOOK LONG WINAPI DetourUpdateThread(_In_ HANDLE hThread); - -// After calling DetourUpdateAllOtherThreads, you should call DetourTransactionCommit(Ex) or DetourTransactionAbort as soon as possible. -// In addition to the DetourAttach(Ex)\DetourDetach(Ex) call, other user operations should not be included between them, -// because other user operations may cause CRT lock competition, and at this time CRT lock may be owned by other threads. + +// After calling DetourUpdateAllOtherThreads, you should call DetourTransactionCommit(Ex) or DetourTransactionAbort as soon as possible. +// In addition to the DetourAttach(Ex)\DetourDetach(Ex) call, other user operations should not be included between them, +// because other user operations may cause CRT lock competition, and at this time CRT lock may be owned by other threads. // Other threads have been suspended at this time, so that may cause deadlock problems LONG WINAPI DetourUpdateThreadEx(_In_ HANDLE hThread, _In_opt_ BOOL fCloseThreadHandleOnDestroyDetourThreadObject /*= TRUE*/); BOOL WINAPI DetourUpdateAllOtherThreads(); @@ -919,6 +919,14 @@ PDETOUR_SYM_INFO DetourLoadImageHlp(VOID); #endif #define _CRT_STDIO_ARBITRARY_WIDE_SPECIFIERS 1 +void DetourAssert(const char* expr, const char* filename, unsigned lineno); +#define Detour_Assert(_Expression) (void)( (!!(_Expression)) || (DetourAssert(#_Expression, __FILE__, __LINE__), 0) ) +#ifdef _DEBUG +#define DETOUR_ASSERT(expr) Detour_Assert(expr) +#else// _DEBUG +#define DETOUR_ASSERT(expr) +#endif// _DEBUG + #ifndef DETOUR_TRACE #if DETOUR_DEBUG #define DETOUR_TRACE(x) printf x @@ -1130,86 +1138,193 @@ BOOL WINAPI DetourVirtualProtectSameExecute(_In_ PVOID pAddress, #endif // __cplusplus //////////////////////////////////////////////////////////// Memory management APIs. +// use these APIs can avoid Use-after-free, Double-free and Over-bound-access memory errors. struct DetourMemory { - DWORD dwHead; +private: + DWORD_PTR dwpHead; size_t nObjectSize; - //T object - //DWORD dwTail; + // T object; + // DWORD_PTR dwpTail[]; private: - DetourMemory(size_t nObjectSize, BOOL bOnlyForGetStructSize) + DetourMemory(size_t nObjectSize, BOOL bOnlyForGetNeedAllocateSize) { - dwHead = 0x12345678; this->nObjectSize = nObjectSize; - if (!bOnlyForGetStructSize) { - *TailPtr() = 0x9ABCDEF0; + if (!bOnlyForGetNeedAllocateSize) { + Fill(); } } -public: - size_t GetObjectSize() const +private: + size_t GetNeedAllocateSize() const { - return nObjectSize; + size_t nNeedAllocedSize = sizeof(DetourMemory) + nObjectSize; + return nNeedAllocedSize; + } +private: + PBYTE GetStructPtr() const + { + PBYTE pStruct = (PBYTE)this; + return pStruct; } size_t GetStructSize() const { - size_t nStructSize = sizeof(DetourMemory) + nObjectSize + sizeof(DWORD); + PBYTE pStruct = GetStructPtr(); + size_t nStructSize = DetourMemSize(pStruct); + Detour_Assert(nStructSize); return nStructSize; } - PBYTE ObjectPtr() + PBYTE GetHeadPtr() const + { + PBYTE pHead = (PBYTE)&dwpHead; + return pHead; + } + size_t GetHeadSize() const { - PBYTE p = ((PBYTE)this) + sizeof(DetourMemory); - return p; + size_t nHeadSize = sizeof(dwpHead); + return nHeadSize; + } +public: + PBYTE GetObjectPtr() + { + PBYTE pStruct = GetStructPtr(); + PBYTE pObject = pStruct + sizeof(DetourMemory); + return pObject; + } + size_t GetObjectSize() const + { + return nObjectSize; } private: - PDWORD TailPtr() + PBYTE GetTailPtr() { - PDWORD pdwTail = (PDWORD)(ObjectPtr() + nObjectSize); - return pdwTail; + PBYTE pObject = GetObjectPtr(); + PBYTE pTail = pObject + nObjectSize; + return pTail; } - BOOL Validate() + size_t GetTailSize() const + { + size_t nTailSize = 0; + size_t nStructSize = GetStructSize(); + if (nStructSize) { + nTailSize = nStructSize - sizeof(DetourMemory) - nObjectSize; + // nTailSize equal to 0 is allowed + } + return nTailSize; + } +private: + BOOL ValidateOrFillFlagPart_Internal(PBYTE pFlagPart, size_t nFlagPartSize, const DWORD_PTR dwpFlagPart_Per, BOOL bValidate) { - BOOL bIsValid = dwHead == 0x12345678; - assert(bIsValid); - if (bIsValid) { - bIsValid = *TailPtr() == 0x9ABCDEF0; - assert(bIsValid); + BOOL bIsValid = TRUE; + DWORD_PTR* dwpFlagPart_Ptr = (DWORD_PTR*)pFlagPart; + size_t dwpFlagPart_Size = nFlagPartSize / sizeof(dwpFlagPart_Per); + for (size_t i = 0; i < dwpFlagPart_Size; i++) { + if (bValidate) { + if (dwpFlagPart_Ptr[i] != dwpFlagPart_Per) { + bIsValid = FALSE; + Detour_Assert(bIsValid); + break; + } + } + else { + dwpFlagPart_Ptr[i] = dwpFlagPart_Per; + } } return bIsValid; } + BOOL ValidateOrFillFlagPart_Head(BOOL bValidate) + { + PBYTE pFlagPart = GetHeadPtr(); + size_t nFlagPartSize = GetHeadSize(); + const DWORD_PTR dwpFlagPart_Per = *(DWORD_PTR*)"HeadPart"; + BOOL bIsValid = ValidateOrFillFlagPart_Internal(pFlagPart, nFlagPartSize, dwpFlagPart_Per, bValidate); + Detour_Assert(bIsValid); + return bIsValid; + } + BOOL ValidateOrFillFlagPart_Tail(BOOL bValidate) + { + PBYTE pFlagPart = GetTailPtr(); + size_t nFlagPartSize = GetTailSize(); + const DWORD_PTR dwpFlagPart_Per = *(DWORD_PTR*)"TailPart"; + BOOL bIsValid = ValidateOrFillFlagPart_Internal(pFlagPart, nFlagPartSize, dwpFlagPart_Per, bValidate); + return bIsValid; + } + BOOL Fill() + { +#ifdef __ENABLE_DETOUR_MEMORY_VALIDATE__ + return ValidateOrFillFlagPart_Head(FALSE) && ValidateOrFillFlagPart_Tail(FALSE); +#else + return TRUE; +#endif + } + BOOL Validate() + { +#ifdef __ENABLE_DETOUR_MEMORY_VALIDATE__ + return ValidateOrFillFlagPart_Head(TRUE) && ValidateOrFillFlagPart_Tail(TRUE); +#else + return TRUE; +#endif + } private: static inline PVOID DetourMemAlloc(size_t size) { - PVOID memblock = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE); - assert(memblock); - return memblock; + PVOID pDetourMemBlock = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE); + return pDetourMemBlock; + } + static inline PVOID DetourMemReAlloc(PVOID pDetourMemBlock_Old, size_t size_New) + { + PVOID pDetourMemBlock_New = VirtualAlloc(pDetourMemBlock_Old, size_New, MEM_COMMIT, PAGE_READWRITE); + if (pDetourMemBlock_New == NULL) { + pDetourMemBlock_New = VirtualAlloc(NULL, size_New, MEM_COMMIT, PAGE_READWRITE); + } + return pDetourMemBlock_New; + } + static inline size_t DetourMemSize(PVOID pDetourMemBlock) + { + MEMORY_BASIC_INFORMATION mbi; + mbi.RegionSize = 0; + BOOL bResult = VirtualQuery(pDetourMemBlock, &mbi, sizeof(mbi)) == sizeof(mbi); + Detour_Assert(bResult); + if (bResult) { + bResult = mbi.BaseAddress == mbi.AllocationBase; + Detour_Assert(bResult); + if (!bResult) { + mbi.RegionSize = 0; + } + } + return mbi.RegionSize; } - static inline BOOL DetourMemFree(PVOID memblock) + static inline BOOL DetourMemFree(PVOID pDetourMemBlock) { - BOOL bResult = VirtualFree(memblock, 0, MEM_RELEASE); - assert(bResult); + BOOL bResult = VirtualFree(pDetourMemBlock, 0, MEM_RELEASE); + Detour_Assert(bResult); return bResult; } public: static DetourMemory* Create(size_t nObjectSize) { DetourMemory m(nObjectSize, TRUE); - DetourMemory* pm = (DetourMemory*)DetourMemAlloc(m.GetStructSize()); - assert(pm); + DetourMemory* pm = (DetourMemory*)DetourMemAlloc(m.GetNeedAllocateSize()); if (pm) { ::new(pm) DetourMemory(m.GetObjectSize(), FALSE); } return pm; } - static BOOL Destroy(DetourMemory*& pm, BOOL bValidate) + static DetourMemory* ReCreate(DetourMemory* pm_Old, size_t nObjectSize_New) + { + DetourMemory m(nObjectSize_New, TRUE); + DetourMemory* pm_New = (DetourMemory*)DetourMemReAlloc(pm_Old, m.GetNeedAllocateSize()); + if (pm_New) { + ::new(pm_New) DetourMemory(m.GetObjectSize(), FALSE); + } + return pm_New; + } + static BOOL Destroy(DetourMemory*& pm) { BOOL bResult = FALSE; if (pm) { - bResult = bValidate ? pm->Validate() : TRUE; + bResult = DetourMemFree((PVOID)pm); if (bResult) { - bResult = DetourMemFree((PVOID)pm); - if (bResult) { - pm = NULL; - } + pm = NULL; } } return bResult; @@ -1228,36 +1343,108 @@ struct DetourMemory } }; +inline PVOID __cdecl DetourObjectByteArray_Alloc(size_t size) +{ + DetourMemory* pm = DetourMemory::Create(size); + BYTE* p = NULL; + if (pm) { + p = pm->GetObjectPtr(); + } + return p; +} +inline void __cdecl DetourObjectByteArray_Free(PVOID p) +{ + DetourMemory* pm = DetourMemory::FromObjectPtr((PBYTE)p); + if (pm) { + DetourMemory::Destroy(pm); + } +} +inline size_t __cdecl DetourObjectByteArray_Size(PVOID p) +{ + size_t size = 0; + DetourMemory* pm = DetourMemory::FromObjectPtr((PBYTE)p); + Detour_Assert(pm); + if (pm) { + size = pm->GetObjectSize(); + } + return size; +} +inline PVOID __cdecl DetourObjectByteArray_ReAlloc(PVOID p_Old, size_t size_New) +{ + if (p_Old == NULL) { + return DetourObjectByteArray_Alloc(size_New); + } + + if (size_New == 0) { + DetourObjectByteArray_Free(p_Old); + return NULL; + } + + DetourMemory* pm_Old = DetourMemory::FromObjectPtr((PBYTE)p_Old); + if (pm_Old == NULL) { + // p_Old is a invalid memory block! + return NULL; + } + DetourMemory* pm_New = DetourMemory::ReCreate(pm_Old, size_New); + BYTE* p_New = NULL; + if (pm_New) { + p_New = pm_New->GetObjectPtr(); + if (p_New != p_Old) { + size_t copy_size = __min(pm_Old->GetObjectSize(), pm_New->GetObjectSize()); + memcpy(p_New, p_Old, copy_size); + DetourObjectByteArray_Free(p_Old); + p_Old = NULL; + } + } + + return p_New; +} + template T* DetourCreateObject() { - DetourMemory* pm = DetourMemory::Create(sizeof(T)); - T* p = NULL; - if (pm) { - p = (T*)pm->ObjectPtr(); + T* p = (T*)DetourObjectByteArray_Alloc(sizeof(T)); + if (p) { ::new(p) T; + // Revalidate DetourMemory object after call its constructor + DetourMemory* pm = DetourMemory::FromObjectPtr((PBYTE)p); + if (pm == NULL) { + // overwrite error + Detour_Assert(pm); + p = NULL; + } } return p; } template T* DetourCreateObject(P1 p1) { - DetourMemory* pm = DetourMemory::Create(sizeof(T)); - T* p = NULL; - if (pm) { - p = (T*)pm->ObjectPtr(); + T* p = (T*)DetourObjectByteArray_Alloc(sizeof(T)); + if (p) { ::new(p) T(p1); + // Revalidate DetourMemory object after call its constructor + DetourMemory* pm = DetourMemory::FromObjectPtr((PBYTE)p); + if (pm == NULL) { + // overwrite error + Detour_Assert(pm); + p = NULL; + } } return p; } template T* DetourCreateObject(P1 p1, P2 p2) { - DetourMemory* pm = DetourMemory::Create(sizeof(T)); - T* p = NULL; - if (pm) { - p = (T*)pm->ObjectPtr(); + T* p = (T*)DetourObjectByteArray_Alloc(sizeof(T)); + if (p) { ::new(p) T(p1, p2); + // Revalidate DetourMemory object after call its constructor + DetourMemory* pm = DetourMemory::FromObjectPtr((PBYTE)p); + if (pm == NULL) { + // overwrite error + Detour_Assert(pm); + p = NULL; + } } return p; } @@ -1267,29 +1454,33 @@ void DetourDestroyObject(T*& p) DetourMemory* pm = DetourMemory::FromObjectPtr((PBYTE)p); if (pm) { size_t MemSize = pm->GetObjectSize(); - assert(MemSize == sizeof(T)); + Detour_Assert(MemSize == sizeof(T)); if (MemSize != sizeof(T)) { return; } p->~T(); + // validate and release DetourMemory object after call their destructor + DetourObjectByteArray_Free((PVOID)p); p = NULL; - - DetourMemory::Destroy(pm, FALSE); } } template -T* DetourCreateObjectArray(size_t _Size) +T* DetourCreateObjectArray(size_t size) { - DetourMemory* pm = DetourMemory::Create(sizeof(T) * _Size); - T* p = NULL; - if (pm) { - p = (T*)pm->ObjectPtr(); - for (size_t i = 0; i < _Size; i++) { + T* p = (T*)DetourObjectByteArray_Alloc(sizeof(T) * size); + if (p) { + for (size_t i = 0; i < size; i++) { ::new(&p[i]) T; } + // Revalidate DetourMemory object after call their constructor + DetourMemory* pm = DetourMemory::FromObjectPtr((PBYTE)p); + if (pm == NULL) { + // overwrite error + Detour_Assert(pm); + p = NULL; + } } - return p; } template @@ -1298,19 +1489,19 @@ void DetourDestroyObjectArray(T*& p) DetourMemory* pm = DetourMemory::FromObjectPtr((PBYTE)p); if (pm) { size_t MemSize = pm->GetObjectSize(); - assert(MemSize % sizeof(T) == 0); + Detour_Assert(MemSize % sizeof(T) == 0); if (MemSize % sizeof(T) != 0) { return; } size_t _Size = MemSize / sizeof(T); - //_Size equal to 0 is allowed + // _Size equal to 0 is allowed for (size_t i = 0; i < _Size; i++) { (&p[i])->~T(); } + // validate and release DetourMemory object after call their destructor + DetourObjectByteArray_Free((PVOID)p); p = NULL; - - DetourMemory::Destroy(pm, FALSE); } } From 2da209b1be840a61e06f6c2687a53575d3af4df0 Mon Sep 17 00:00:00 2001 From: sonyps5201314 Date: Mon, 25 Jan 2021 22:36:45 +0800 Subject: [PATCH 09/14] refine the code --- src/detours.cpp | 369 +++++++++++++++++++----------------------------- 1 file changed, 142 insertions(+), 227 deletions(-) diff --git a/src/detours.cpp b/src/detours.cpp index 8caa35c6..445844d5 100644 --- a/src/detours.cpp +++ b/src/detours.cpp @@ -10,6 +10,9 @@ //#define DETOUR_DEBUG 1 #define DETOURS_INTERNAL +typedef long NTSTATUS; +#include +#define WIN32_NO_STATUS #include "detours.h" #if DETOURS_VERSION != 0x4c0c1 // 0xMAJORcMINORcPATCH @@ -2020,276 +2023,188 @@ LONG WINAPI DetourUpdateThreadEx(_In_ HANDLE hThread, _In_opt_ BOOL fCloseThread return NO_ERROR; } -// Code modified from https://github.com/apriorit/mhook/blob/master/mhook-lib/mhook.c -//========================================================================= -// ntdll definitions - -typedef LONG NTSTATUS; +// winternl.h is different in each version of the Windows SDK, +// and some fields or types we need are missing in the lower version, +// so we do not use winternl.h in the current compilation environment by default. +#ifdef __USE_WINTERNL_H__ +#include +#else// __USE_WINTERNL_H__ +// Excerpted from "C:\Program Files (x86)\Windows Kits\10\Include\10.0.19041.0\um\winternl.h" +typedef struct _UNICODE_STRING { + USHORT Length; + USHORT MaximumLength; + PWSTR Buffer; +} UNICODE_STRING; +typedef UNICODE_STRING* PUNICODE_STRING; +typedef const UNICODE_STRING* PCUNICODE_STRING; typedef LONG KPRIORITY; -typedef enum _SYSTEM_INFORMATION_CLASS -{ - SystemBasicInformation = 0, - SystemPerformanceInformation = 2, - SystemTimeOfDayInformation = 3, - SystemProcessInformation = 5, - SystemProcessorPerformanceInformation = 8, - SystemHandleInformation = 16, - SystemInterruptInformation = 23, - SystemExceptionInformation = 33, - SystemRegistryQuotaInformation = 37, - SystemLookasideInformation = 45, - SystemProcessIdInformation = 0x58 -} SYSTEM_INFORMATION_CLASS; +typedef struct _CLIENT_ID { + HANDLE UniqueProcess; + HANDLE UniqueThread; +} CLIENT_ID; -typedef enum _KWAIT_REASON -{ - Executive, - FreePage, - PageIn, - PoolAllocation, - DelayExecution, - Suspended, - UserRequest, - WrExecutive, - WrFreePage, - WrPageIn, - WrPoolAllocation, - WrDelayExecution, - WrSuspended, - WrUserRequest, - WrEventPair, - WrQueue, - WrLpcReceive, - WrLpcReply, - WrVirtualMemory, - WrPageOut, - WrRendezvous, - Spare2, - Spare3, - Spare4, - Spare5, - Spare6, - WrKernel, - MaximumWaitReason -} KWAIT_REASON, * PKWAIT_REASON; - -typedef struct _CLIENT_ID -{ - HANDLE UniqueProcess; - HANDLE UniqueThread; -} CLIENT_ID, * PCLIENT_ID; - -typedef struct _SYSTEM_THREAD_INFORMATION -{ - LARGE_INTEGER KernelTime; - LARGE_INTEGER UserTime; - LARGE_INTEGER CreateTime; - ULONG WaitTime; +typedef struct _SYSTEM_THREAD_INFORMATION { + LARGE_INTEGER Reserved1[3]; + ULONG Reserved2; PVOID StartAddress; CLIENT_ID ClientId; KPRIORITY Priority; LONG BasePriority; - ULONG ContextSwitches; + ULONG Reserved3; ULONG ThreadState; - KWAIT_REASON WaitReason; + ULONG WaitReason; } SYSTEM_THREAD_INFORMATION, * PSYSTEM_THREAD_INFORMATION; -typedef struct _UNICODE_STRING -{ - USHORT Length; - USHORT MaximumLength; - PWSTR Buffer; -} UNICODE_STRING; - -typedef struct _SYSTEM_PROCESS_INFORMATION -{ - ULONG uNext; - ULONG uThreadCount; - LARGE_INTEGER WorkingSetPrivateSize; // since VISTA - ULONG HardFaultCount; // since WIN7 - ULONG NumberOfThreadsHighWatermark; // since WIN7 - ULONGLONG CycleTime; // since WIN7 - LARGE_INTEGER CreateTime; - LARGE_INTEGER UserTime; - LARGE_INTEGER KernelTime; +typedef struct _SYSTEM_PROCESS_INFORMATION { + ULONG NextEntryOffset; + ULONG NumberOfThreads; + BYTE Reserved1[48]; UNICODE_STRING ImageName; KPRIORITY BasePriority; - HANDLE uUniqueProcessId; - HANDLE InheritedFromUniqueProcessId; + HANDLE UniqueProcessId; + PVOID Reserved2; ULONG HandleCount; ULONG SessionId; - ULONG_PTR UniqueProcessKey; // since VISTA (requires SystemExtendedProcessInformation) + PVOID Reserved3; SIZE_T PeakVirtualSize; SIZE_T VirtualSize; - ULONG PageFaultCount; + ULONG Reserved4; SIZE_T PeakWorkingSetSize; SIZE_T WorkingSetSize; - SIZE_T QuotaPeakPagedPoolUsage; + PVOID Reserved5; SIZE_T QuotaPagedPoolUsage; - SIZE_T QuotaPeakNonPagedPoolUsage; + PVOID Reserved6; SIZE_T QuotaNonPagedPoolUsage; SIZE_T PagefileUsage; SIZE_T PeakPagefileUsage; SIZE_T PrivatePageCount; - LARGE_INTEGER ReadOperationCount; - LARGE_INTEGER WriteOperationCount; - LARGE_INTEGER OtherOperationCount; - LARGE_INTEGER ReadTransferCount; - LARGE_INTEGER WriteTransferCount; - LARGE_INTEGER OtherTransferCount; - SYSTEM_THREAD_INFORMATION Threads[1]; + LARGE_INTEGER Reserved7[6]; } SYSTEM_PROCESS_INFORMATION, * PSYSTEM_PROCESS_INFORMATION; -//========================================================================= -// ZwQuerySystemInformation definitions -typedef NTSTATUS(NTAPI* PZwQuerySystemInformation)( - __in SYSTEM_INFORMATION_CLASS SystemInformationClass, - __inout PVOID SystemInformation, - __in ULONG SystemInformationLength, - __out_opt PULONG ReturnLength - ); - -#define STATUS_INFO_LENGTH_MISMATCH ((NTSTATUS)0xC0000004L) -static PZwQuerySystemInformation fnZwQuerySystemInformation = NULL; -//========================================================================= -// Internal function: -// -// Get snapshot of the processes started in the system -//========================================================================= -static BOOL CreateProcessSnapshot(PBYTE* snapshotContext) -{ - HMODULE hModule = NULL; - ULONG cbBuffer = 1024 * 1024; // 1Mb - default process information buffer size (that's enough in most cases for high-loaded systems) - PBYTE pBuffer = NULL; - NTSTATUS status = 0; - - if (!fnZwQuerySystemInformation) { - hModule = GetModuleHandle(TEXT("ntdll.dll")); - if (!hModule) { - return FALSE; - } - fnZwQuerySystemInformation = (PZwQuerySystemInformation)GetProcAddress(hModule, "ZwQuerySystemInformation"); - if (!fnZwQuerySystemInformation) { - return FALSE; - } - } +typedef enum _SYSTEM_INFORMATION_CLASS { + SystemBasicInformation = 0, + SystemPerformanceInformation = 2, + SystemTimeOfDayInformation = 3, + SystemProcessInformation = 5, + SystemProcessorPerformanceInformation = 8, + SystemInterruptInformation = 23, + SystemExceptionInformation = 33, + SystemRegistryQuotaInformation = 37, + SystemLookasideInformation = 45, + SystemCodeIntegrityInformation = 103, + SystemPolicyInformation = 134, +} SYSTEM_INFORMATION_CLASS; - do { - pBuffer = DetourCreateObjectArray(cbBuffer); - if (pBuffer == NULL) { - return FALSE; - } +// +// Generic test for success on any status value (non-negative numbers +// indicate success). +// - status = fnZwQuerySystemInformation(SystemProcessInformation, pBuffer, cbBuffer, NULL); +#ifndef NT_SUCCESS +#define NT_SUCCESS(Status) (((NTSTATUS)(Status)) >= 0) +#endif - if (status == STATUS_INFO_LENGTH_MISMATCH) { - DetourDestroyObjectArray(pBuffer); - cbBuffer *= 2; - } - else if (status < 0) { - DetourDestroyObjectArray(pBuffer); - return FALSE; - } - } while (status == STATUS_INFO_LENGTH_MISMATCH); +#endif// __USE_WINTERNL_H__ - *snapshotContext = pBuffer; +static +NTSTATUS +(NTAPI* +pfnNtQuerySystemInformation)( + IN SYSTEM_INFORMATION_CLASS SystemInformationClass, + OUT PVOID SystemInformation, + IN ULONG SystemInformationLength, + OUT PULONG ReturnLength OPTIONAL +); - return TRUE; -} - -//========================================================================= -// Internal function: -// -// Find and return process information from snapshot -//========================================================================= -static PSYSTEM_PROCESS_INFORMATION FindProcess(VOID* snapshotContext, SIZE_T processId) +BOOL WINAPI DetourUpdateAllOtherThreads() { - PSYSTEM_PROCESS_INFORMATION currentProcess = (PSYSTEM_PROCESS_INFORMATION)snapshotContext; - - while (currentProcess != NULL) { - if (currentProcess->uUniqueProcessId == (HANDLE)processId) { - break; - } + LONG bResult = FALSE; - if (currentProcess->uNext == 0) { - currentProcess = NULL; + if (!pfnNtQuerySystemInformation) { + HMODULE hmod_ntdll = GetModuleHandle(TEXT("ntdll.dll")); + DETOUR_ASSERT(hmod_ntdll); + if (!hmod_ntdll) { + return bResult; } - else { - currentProcess = (PSYSTEM_PROCESS_INFORMATION)(((LPBYTE)currentProcess) + currentProcess->uNext); + pfnNtQuerySystemInformation = (NTSTATUS(NTAPI*)(SYSTEM_INFORMATION_CLASS, PVOID, ULONG, PULONG))GetProcAddress(hmod_ntdll, "NtQuerySystemInformation"); + DETOUR_ASSERT(pfnNtQuerySystemInformation); + if (!pfnNtQuerySystemInformation) { + return bResult; } } - return currentProcess; -} - -//========================================================================= -// Internal function: -// -// Get current process snapshot and process info -// -//========================================================================= -static BOOL GetCurrentProcessSnapshot(PBYTE* snapshot, PSYSTEM_PROCESS_INFORMATION* procInfo) -{ - // get a view of the threads in the system - - if (!CreateProcessSnapshot(snapshot)) { - DETOUR_TRACE(("detours: can't get process snapshot!")); - return FALSE; - } - - // this never returns NULL as the current process id is always present in the processes snapshot - *procInfo = FindProcess(*snapshot, GetCurrentProcessId()); - DETOUR_ASSERT(procInfo); - - return TRUE; -} - -//========================================================================= -// Internal function: -// -// Free memory allocated for processes snapshot -//========================================================================= -static VOID CloseProcessSnapshot(PBYTE& snapshotContext) -{ - DetourDestroyObjectArray(snapshotContext); -} - -BOOL WINAPI DetourUpdateAllOtherThreads() -{ - LONG bResult = FALSE; + ULONG data_size = 1024 * 1024; + for (int i = 0; i < 10; i++) { + PBYTE data = DetourCreateObjectArray(data_size); + if (data == NULL) { + break; + } - PBYTE procEnumerationCtx = NULL; - PSYSTEM_PROCESS_INFORMATION procInfo = NULL; - - if (GetCurrentProcessSnapshot(&procEnumerationCtx, &procInfo)) { - DWORD tid = GetCurrentThreadId(); - // go through every thread - for (ULONG threadIdx = 0; threadIdx < procInfo->uThreadCount; threadIdx++) { - DWORD threadId = (DWORD)(DWORD_PTR)procInfo->Threads[threadIdx].ClientId.UniqueThread; - - if (threadId != tid) { - HANDLE hThread = OpenThread(THREAD_SUSPEND_RESUME | THREAD_GET_CONTEXT | THREAD_QUERY_INFORMATION | THREAD_SET_CONTEXT, FALSE, threadId); - if (hThread) { - LONG error = DetourUpdateThreadEx(hThread, TRUE); - DETOUR_ASSERT(error == NO_ERROR); - if (error) { - DETOUR_TRACE(("DetourUpdateThreadEx failed, error=%d\n", error)); - // Reset s_nPendingError so that subsequent threads can continue to try to call the DetourUpdateThreadEx function - DETOUR_ASSERT(error == s_nPendingError); - // When the console program is terminated, s_nPendingError == ERROR_ACCESS_DENIED may be caused when the SuspendThread is called in DetourUpdateThreadEx - DETOUR_ASSERT(s_nPendingError == ERROR_ACCESS_DENIED); - s_nPendingError = NO_ERROR; + NTSTATUS status = pfnNtQuerySystemInformation(SystemProcessInformation, data, data_size, NULL); + if (NT_SUCCESS(status)) { + __if_not_exists(SYSTEM_PROCESS_INFORMATION::Threads) { + struct MySYSTEM_PROCESS_INFORMATION :public SYSTEM_PROCESS_INFORMATION { + SYSTEM_THREAD_INFORMATION Threads[1]; + }; + } + __if_exists(SYSTEM_PROCESS_INFORMATION::Threads) { + struct MySYSTEM_PROCESS_INFORMATION :public SYSTEM_PROCESS_INFORMATION { + }; + } + MySYSTEM_PROCESS_INFORMATION* pSYSTEM_PROCESS_INFORMATIONs = (MySYSTEM_PROCESS_INFORMATION*)data; + DWORD pid = GetCurrentProcessId(); + while (pSYSTEM_PROCESS_INFORMATIONs) { + if (pSYSTEM_PROCESS_INFORMATIONs->UniqueProcessId == (HANDLE)(DWORD_PTR)pid) { + DWORD tid = GetCurrentThreadId(); + for (ULONG threadIdx = 0; threadIdx < pSYSTEM_PROCESS_INFORMATIONs->NumberOfThreads; threadIdx++) { + DWORD threadId = (DWORD)(DWORD_PTR)pSYSTEM_PROCESS_INFORMATIONs->Threads[threadIdx].ClientId.UniqueThread; + + if (threadId != tid) { + HANDLE hThread = OpenThread(THREAD_SUSPEND_RESUME | THREAD_GET_CONTEXT | THREAD_QUERY_INFORMATION | THREAD_SET_CONTEXT, FALSE, threadId); + if (hThread) { + LONG error = DetourUpdateThreadEx(hThread, TRUE); + DETOUR_ASSERT(error == NO_ERROR); + if (error) { + DETOUR_TRACE(("DetourUpdateThreadEx failed, error=%d\n", error)); + // Reset s_nPendingError so that subsequent threads can continue to try to call the DetourUpdateThreadEx function + DETOUR_ASSERT(error == s_nPendingError); + // When the console program is terminated, s_nPendingError == ERROR_ACCESS_DENIED may be caused when the SuspendThread is called in DetourUpdateThreadEx + DETOUR_ASSERT(s_nPendingError == ERROR_ACCESS_DENIED); + s_nPendingError = NO_ERROR; + } + } + } } + bResult = TRUE; + break; + } + else if (pSYSTEM_PROCESS_INFORMATIONs->NextEntryOffset) { + pSYSTEM_PROCESS_INFORMATIONs = (MySYSTEM_PROCESS_INFORMATION*)(((LPBYTE)pSYSTEM_PROCESS_INFORMATIONs) + pSYSTEM_PROCESS_INFORMATIONs->NextEntryOffset); + } + else { + pSYSTEM_PROCESS_INFORMATIONs = NULL; } } - } - CloseProcessSnapshot(procEnumerationCtx); + DetourDestroyObjectArray(data); + data = NULL; + break; + } + else { + DetourDestroyObjectArray(data); + data = NULL; - bResult = TRUE; - } + DETOUR_ASSERT(status == STATUS_INFO_LENGTH_MISMATCH || status == STATUS_BUFFER_TOO_SMALL); + if (status == STATUS_INFO_LENGTH_MISMATCH || status == STATUS_BUFFER_TOO_SMALL) { + data_size *= 2; + } + else { + break; + } + } + }; return bResult; } From e5ce233735a9e9cf2cad81d7530d7cde4e77cec7 Mon Sep 17 00:00:00 2001 From: sonyps5201314 Date: Mon, 25 Jan 2021 23:51:02 +0800 Subject: [PATCH 10/14] The code format is adapted to the Detours code style --- src/detours.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/detours.cpp b/src/detours.cpp index 445844d5..492a2a88 100644 --- a/src/detours.cpp +++ b/src/detours.cpp @@ -2028,7 +2028,7 @@ LONG WINAPI DetourUpdateThreadEx(_In_ HANDLE hThread, _In_opt_ BOOL fCloseThread // so we do not use winternl.h in the current compilation environment by default. #ifdef __USE_WINTERNL_H__ #include -#else// __USE_WINTERNL_H__ +#else // __USE_WINTERNL_H__ // Excerpted from "C:\Program Files (x86)\Windows Kits\10\Include\10.0.19041.0\um\winternl.h" typedef struct _UNICODE_STRING { USHORT Length; @@ -2106,7 +2106,7 @@ typedef enum _SYSTEM_INFORMATION_CLASS { #define NT_SUCCESS(Status) (((NTSTATUS)(Status)) >= 0) #endif -#endif// __USE_WINTERNL_H__ +#endif // __USE_WINTERNL_H__ static NTSTATUS From 9d415340437c9b9e173af94707b2e32a07937a78 Mon Sep 17 00:00:00 2001 From: sonyps5201314 Date: Thu, 28 Jan 2021 13:13:25 +0800 Subject: [PATCH 11/14] Fix a warning in code analysis --- src/detours.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/detours.h b/src/detours.h index 76be688a..90f09e94 100644 --- a/src/detours.h +++ b/src/detours.h @@ -1149,6 +1149,7 @@ struct DetourMemory private: DetourMemory(size_t nObjectSize, BOOL bOnlyForGetNeedAllocateSize) { + dwpHead = 0; this->nObjectSize = nObjectSize; if (!bOnlyForGetNeedAllocateSize) { Fill(); From 0e0a3a07b01b5f841a4553940b93ed633d1c52bc Mon Sep 17 00:00:00 2001 From: sonyps5201314 Date: Sat, 6 Mar 2021 16:40:55 +0800 Subject: [PATCH 12/14] fix build --- src/detours.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/detours.cpp b/src/detours.cpp index cdad176d..6a16ff45 100644 --- a/src/detours.cpp +++ b/src/detours.cpp @@ -2167,7 +2167,7 @@ BOOL WINAPI DetourUpdateAllOtherThreads() LONG error = DetourUpdateThreadEx(hThread, TRUE); DETOUR_ASSERT(error == NO_ERROR); if (error) { - DETOUR_TRACE(("DetourUpdateThreadEx failed, error=%d\n", error)); + DETOUR_TRACE(("DetourUpdateThreadEx failed, error=%ld\n", error)); // Reset s_nPendingError so that subsequent threads can continue to try to call the DetourUpdateThreadEx function DETOUR_ASSERT(error == s_nPendingError); // When the console program is terminated, s_nPendingError == ERROR_ACCESS_DENIED may be caused when the SuspendThread is called in DetourUpdateThreadEx From e8eb6b78c866a5cfc89633a333f818b4af1c5435 Mon Sep 17 00:00:00 2001 From: sonyps5201314 Date: Sun, 21 Nov 2021 00:49:40 +0800 Subject: [PATCH 13/14] Fix an assertion error --- src/detours.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/detours.cpp b/src/detours.cpp index a99d33fe..993d69f1 100644 --- a/src/detours.cpp +++ b/src/detours.cpp @@ -1551,8 +1551,6 @@ struct DetourOperation ULONG dwPerm; }; -static HANDLE s_hHeap = NULL; - static BOOL s_fIgnoreTooSmall = FALSE; static BOOL s_fRetainRegions = FALSE; @@ -2165,7 +2163,7 @@ BOOL WINAPI DetourUpdateAllOtherThreads() HANDLE hThread = OpenThread(THREAD_SUSPEND_RESUME | THREAD_GET_CONTEXT | THREAD_QUERY_INFORMATION | THREAD_SET_CONTEXT, FALSE, threadId); if (hThread) { LONG error = DetourUpdateThreadEx(hThread, TRUE); - DETOUR_ASSERT(error == NO_ERROR); + DETOUR_ASSERT(error == NO_ERROR || error == ERROR_ACCESS_DENIED); if (error) { DETOUR_TRACE(("DetourUpdateThreadEx failed, error=%ld\n", error)); // Reset s_nPendingError so that subsequent threads can continue to try to call the DetourUpdateThreadEx function From 3d2d13b78cc7a17b9c3167f6209b6045b83127f8 Mon Sep 17 00:00:00 2001 From: sonyps5201314 Date: Mon, 1 Aug 2022 10:55:47 +0800 Subject: [PATCH 14/14] Maintenance: Fix incorrect SAL annotation found by latest MSVC Analysis warning C6553: The annotation for function 'DetourTransactionBeginEx' on _Param_(1) does not apply to a value type. warning C6553: The annotation for function 'DetourUpdateThreadEx' on _Param_(2) does not apply to a value type. Flagged By: VS 17.2.6 (CL.exe 14.32.31332.0) --- src/detours.cpp | 4 ++-- src/detours.h | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/detours.cpp b/src/detours.cpp index 426aa7c7..e90e6d61 100644 --- a/src/detours.cpp +++ b/src/detours.cpp @@ -1610,7 +1610,7 @@ LONG WINAPI DetourTransactionBegin() return DetourTransactionBeginEx(FALSE); } -LONG WINAPI DetourTransactionBeginEx(_In_opt_ BOOL fWait) +LONG WINAPI DetourTransactionBeginEx(_In_ BOOL fWait) { // Only one transaction is allowed at a time. _Benign_race_begin_ @@ -1986,7 +1986,7 @@ LONG WINAPI DetourUpdateThread(_In_ HANDLE hThread) return DetourUpdateThreadEx(hThread, FALSE); } -LONG WINAPI DetourUpdateThreadEx(_In_ HANDLE hThread, _In_opt_ BOOL fCloseThreadHandleOnDestroyDetourThreadObject) +LONG WINAPI DetourUpdateThreadEx(_In_ HANDLE hThread, _In_ BOOL fCloseThreadHandleOnDestroyDetourThreadObject) { LONG error; diff --git a/src/detours.h b/src/detours.h index a43a548a..9dd514d2 100644 --- a/src/detours.h +++ b/src/detours.h @@ -383,10 +383,10 @@ extern const GUID DETOUR_EXE_RESTORE_GUID; extern const GUID DETOUR_EXE_HELPER_GUID; #define DETOUR_TRAMPOLINE_SIGNATURE 0x21727444 // Dtr! -typedef struct _DETOUR_TRAMPOLINE DETOUR_TRAMPOLINE, *PDETOUR_TRAMPOLINE; - -#ifndef DETOUR_MAX_SUPPORTED_IMAGE_SECTION_HEADERS -#define DETOUR_MAX_SUPPORTED_IMAGE_SECTION_HEADERS 32 +typedef struct _DETOUR_TRAMPOLINE DETOUR_TRAMPOLINE, *PDETOUR_TRAMPOLINE; + +#ifndef DETOUR_MAX_SUPPORTED_IMAGE_SECTION_HEADERS +#define DETOUR_MAX_SUPPORTED_IMAGE_SECTION_HEADERS 32 #endif // !DETOUR_MAX_SUPPORTED_IMAGE_SECTION_HEADERS /////////////////////////////////////////////////////////// Binary Structures. @@ -559,7 +559,7 @@ typedef VOID * PDETOUR_LOADED_BINARY; //////////////////////////////////////////////////////////// Transaction APIs. // LONG WINAPI DetourTransactionBegin(VOID); -LONG WINAPI DetourTransactionBeginEx(_In_opt_ BOOL fWait /*= TRUE*/); +LONG WINAPI DetourTransactionBeginEx(_In_ BOOL fWait /*= TRUE*/); LONG WINAPI DetourTransactionAbort(VOID); LONG WINAPI DetourTransactionCommit(VOID); LONG WINAPI DetourTransactionCommitEx(_Out_opt_ PVOID **pppFailedPointer); @@ -571,7 +571,7 @@ LONG WINAPI DetourUpdateThread(_In_ HANDLE hThread); // In addition to the DetourAttach(Ex)\DetourDetach(Ex) call, other user operations should not be included between them, // because other user operations may cause CRT lock competition, and at this time CRT lock may be owned by other threads. // Other threads have been suspended at this time, so that may cause deadlock problems -LONG WINAPI DetourUpdateThreadEx(_In_ HANDLE hThread, _In_opt_ BOOL fCloseThreadHandleOnDestroyDetourThreadObject /*= TRUE*/); +LONG WINAPI DetourUpdateThreadEx(_In_ HANDLE hThread, _In_ BOOL fCloseThreadHandleOnDestroyDetourThreadObject /*= TRUE*/); BOOL WINAPI DetourUpdateAllOtherThreads(); LONG WINAPI DetourAttach(_Inout_ PVOID *ppPointer, @@ -605,8 +605,8 @@ PVOID WINAPI DetourCopyInstruction(_In_opt_ PVOID pDst, BOOL WINAPI DetourSetCodeModule(_In_ HMODULE hModule, _In_ BOOL fLimitReferencesToModule); PVOID WINAPI DetourAllocateRegionWithinJumpBounds(_In_ LPCVOID pbTarget, - _Out_ PDWORD pcbAllocatedSize); -BOOL WINAPI DetourIsFunctionImported(_In_ PBYTE pbCode, + _Out_ PDWORD pcbAllocatedSize); +BOOL WINAPI DetourIsFunctionImported(_In_ PBYTE pbCode, _In_ PBYTE pbAddress); ///////////////////////////////////////////////////// Loaded Binary Functions.