Stack overflow crash

c++ / delphi package - dll injection and api hooking
Post Reply
EaSy
Posts: 150
Joined: Tue Oct 23, 2012 12:33 pm

Stack overflow crash

Post by EaSy »

Hi,
we have found another issue with the checkhooks code. As you remember we were discussing similar issue in viewtopic.php?f=7&t=27969. This time it is even more complicated I believe :wink:.

To be clear we are using the latest MCH version 3.1.9.

The main reason, why I am writing you, is that the application crashes in the endless recursion in one of our hooks. It happened once...
The problem is that the address of the next hook doesn't point to the MCHNextHookStub produced in HookApi function, but to the original address we filled it with during the initialization.

The situation looks like this (from the dump):
The right case:

Code: Select all

0:042> dt SFunctionToBeHooked 00007ffc`769b7eb0+0xf0*0n11
STGuard64!SFunctionToBeHooked
   +0x000 eFunctionIdx     : b ( EFTBH_NTDLL_NTQUERYKEY )
   +0x008 LibraryList      : [12] SLibListEntry
   +0x0c8 FunctionName     : 0x00007ffc`76851598  "NtQueryKey"
   +0x0d0 FunctionNameW    : 0x00007ffc`76851580  "NtQueryKey"
   +0x0d8 FunctionCallbacksPointer : 0x00007ffc`76971900 [2]  -> 0x00007ffc`766d3710 Void
   +0x0e0 pGetProcAddress  : (null) 
   +0x0e8 Flags            : 0n0
   +0x0ec IsHooked         : 1
   +0x0ed IsValid          : 1
0:042> dq 0x00007ffc`76971900
//FCPtr            [0]               [1]
00007ffc`76971900  00007ffc`766d3710 00007ffc`88fb0000
0:042> ln 00007ffc`88fb0000 //ln returned nothing, so it is really MCHNextHookStub
The wrong case:

Code: Select all

0:042> dt SFunctionToBeHooked 00007ffc`769b7eb0+0xf0*0n98
STGuard64!SFunctionToBeHooked
   +0x000 eFunctionIdx     : 62 ( EFTBH_WININET_INTERNETCONNECTW )
   +0x008 LibraryList      : [12] SLibListEntry
   +0x0c8 FunctionName     : 0x00007ffc`76844e00  "InternetConnectW"
   +0x0d0 FunctionNameW    : 0x00007ffc`76844dd8  "InternetConnectW"
   +0x0d8 FunctionCallbacksPointer : 0x00007ffc`7696df60 [2]  -> 0x00007ffc`766d8990 Void
   +0x0e0 pGetProcAddress  : (null) 
   +0x0e8 Flags            : 0n0
   +0x0ec IsHooked         : 1
   +0x0ed IsValid          : 1
0:042> dq 0x00007ffc`7696df60
//FCPtr            [0]               [1]
00007ffc`7696df60  00007ffc`766d8990 00007ffc`7ee4e270
0:042> ln 00007ffc`7ee4e270 //ln returned the original address!! WRONG!! Where is the MCHNextHookStub?
(00007ffc`7ee4e270)   wininet!InternetConnectW   |  (00007ffc`7ee4e4a0)   wininet!InternalInternetConnectA
Exact matches:
    wininet!InternetConnectW (void)
For both functions we call something like:

Code: Select all

HookAPI(GetLibNameA(FunctionToBeHooked->LibraryList),
		  FunctionToBeHooked->FunctionName,
		  (*FunctionToBeHooked->FunctionCallbacksPointer)[0],
		  &((*FunctionToBeHooked->FunctionCallbacksPointer)[1]),
		  FunctionToBeHooked->Flags);
MCH did not modified the nexthook address or modified it in the wrong way.
The difference between those two functions is that the wininet is not linked to our DLL so, unlike the ntdll, it wasn't loaded in the process during the HookAPI call.

I also took a look into the internals of MCH to help you.

first lets dump a g_pHookCollection

Code: Select all

0:042> dt STGuard64!g_pHookCollection
0x00000000`026f86a0 
   +0x000 mpArray          : 0x00000000`0271eb00 tagHookItem
   +0x008 mCount           : 0n40
   +0x00c mSize            : 0n64
   +0x010 mSizeFixed       : 0
0:042> ?? sizeof(tagHookItem)
unsigned int64 0x240
So g_pHookCollection contains 40 tagHookItems.

The first interesting thing is that the mpArray[0] contains an empty entry. Is it a problem?
Maybe a product of HookCode call?

Code: Select all

0:042> dt tagHookItem 0x00000000`0271eb00+0x240*0n0 -r
STGuard64!tagHookItem
   +0x000 hOwner           : (null) 
   +0x008 hModule          : (null) 
   +0x010 ModuleName       : [260]  ""
   +0x114 ProcName         : [260]  ""
   +0x218 pCode            : 0x00007ffc`85fe0000 Void
   +0x220 pCodeHook        : 0x00000000`0270fef0 CCodeHook
      +0x000 __VFN_table : 0x00007ffc`7670d0a0 
      +0x008 mpInUseArray     : 0x00007ffc`86000052 tagInUse
         +0x000 push             : 0x35ff
         +0x002 pushParam        : 0xa
         +0x006 lock             : 0xf0 ''
         +0x007 rex              : 0x48 'H'
         +0x008 and              : 0x2583
         +0x00a lockParam        : 1
         +0x00e zero             : 0 ''
         +0x00f ret              : 0xc3 ''
         +0x010 returnAddress    : 0x00007ffc`85ec8e4a Void
      +0x010 mLeakUnhook      : ffffffffffffffd2
      +0x018 mPatchAddr       : 0x00007ffc`85fe0000 tagAbsoluteJmp
         +0x000 Opcode           : 0xff ''
         +0x001 modRm            : 0x25 'Unknown format characterUnknown format control character
         +0x002 Target           : 0x30030
      +0x020 mNewCode         : tagAbsoluteJmp
         +0x000 Opcode           : 0xff ''
         +0x001 modRm            : 0x25 'Unknown format characterUnknown format control character
         +0x002 Target           : 0x30030
      +0x026 mOldCode         : tagAbsoluteJmp
         +0x000 Opcode           : 0xff ''
         +0x001 modRm            : 0x25 'Unknown format characterUnknown format control character
         +0x002 Target           : 0
      +0x030 mpHookQueue      : 0x00000000`02706c10 CHookQueue
         +0x000 mMapName         : SString
         +0x020 mHMap            : 0x00000000`000001d8 Void
         +0x028 mpMapBuffer      : 0x00000000`00b10000  -> 0x000007fe`ffff0000 Void
         +0x030 mpBuffer         : 0x000007fe`ffff0000 Void
         +0x038 mpHookQueueHeader : 0x000007fe`ffff0000 tagHookQueueHeader
         +0x040 mpHookRecords    : 0x000007fe`ffff0028 tagHookQueueRecord
      +0x038 mHModule         : (null) 
      +0x040 mSafeHooking     : 0n0
      +0x044 mNoImproveUnhook : 0n0
      +0x048 mValid           : 1
      +0x049 mNewHook         : 1
      +0x04a mDestroying      : ffffffffffffffec
      +0x04b mIsWinsock2      : 0
      +0x04c mPatchExportTable : 0
      +0x050 mpHookedFunction : 0x00007ffc`85fe0000 Void
      +0x058 mpCallbackFunction : 0x00007ffc`8600000e Void
      +0x060 mpShareCode      : (null) 
      +0x068 mpNextHook       : 0x00007ffc`769c3e68  -> 0x00007ffc`86000000 Void
      +0x070 mpTramp          : 0x00007ffc`86010000 Void
      +0x078 mpHookStub       : 0x00007ffc`86000000 tagIndirectAbsoluteJump
         +0x000 Opcode           : 0xff ''
         +0x001 modRm            : 0x25 'Unknown format characterUnknown format control character
         +0x002 Target           : 0
         +0x006 AbsoluteAddress  : 0x00007ffc`86010000 Void
      +0x080 mpSparePage      : (null) 
   +0x228 pNextHook        : 0x00007ffc`769c3e68  -> 0x00007ffc`86000000 Void
   +0x230 pCallback        : 0x00007ffc`76584230 Void
   +0x238 Flags            : 0
Ok, lets go to inspect the actual InternetConnectW hooking. With ln used on pCallback and pNextHook

Code: Select all

0:042> dt tagHookItem 0x00000000`0271eb00+0x240*0n39 -r
STGuard64!tagHookItem
   +0x000 hOwner           : 0x00007ffc`764e0000 HINSTANCE__
      +0x000 unused           : 0n9460301
   +0x008 hModule          : 0x00007ffc`7edd0000 HINSTANCE__
      +0x000 unused           : 0n9460301
   +0x010 ModuleName       : [260]  "wininet.dll"
   +0x114 ProcName         : [260]  "InternetConnectW"
   +0x218 pCode            : 0x00007ffc`7ee4e270 Void
   +0x220 pCodeHook        : 0x00000000`02729390 CCodeHook
      +0x000 __VFN_table : 0x00007ffc`7670d0a0 
      +0x008 mpInUseArray     : 0x00007ffc`7f4d0052 tagInUse
         +0x000 push             : 0x35ff
         +0x002 pushParam        : 0xa
         +0x006 lock             : 0xf0 ''
         +0x007 rex              : 0x48 'H'
         +0x008 and              : 0x2583
         +0x00a lockParam        : 1
         +0x00e zero             : 0 ''
         +0x00f ret              : 0xc3 ''
         +0x010 returnAddress    : 0x00007ffc`82117476 Void
      +0x010 mLeakUnhook      : 0
      +0x018 mPatchAddr       : 0x00007ffc`7ee4e270 tagAbsoluteJmp
         +0x000 Opcode           : 0xff ''
         +0x001 modRm            : 0x25 'Unknown format characterUnknown format control character
         +0x002 Target           : 0x6b1dc0
      +0x020 mNewCode         : tagAbsoluteJmp
         +0x000 Opcode           : 0xff ''
         +0x001 modRm            : 0x25 'Unknown format characterUnknown format control character
         +0x002 Target           : 0x6b1dc0
      +0x026 mOldCode         : tagAbsoluteJmp
         +0x000 Opcode           : 0x48 'H'
         +0x001 modRm            : 0x8b ''
         +0x002 Target           : 0x588948c4
      +0x030 mpHookQueue      : 0x00000000`02709d90 CHookQueue
         +0x000 mMapName         : SString
         +0x020 mHMap            : (null) 
         +0x028 mpMapBuffer      : 0x00000000`0d780000  -> 0x000007fe`ffd80000 Void
         +0x030 mpBuffer         : 0x000007fe`ffd80000 Void
         +0x038 mpHookQueueHeader : 0x000007fe`ffd80000 tagHookQueueHeader
         +0x040 mpHookRecords    : 0x000007fe`ffd80028 tagHookQueueRecord
      +0x038 mHModule         : 0x00007ffc`7edd0000 HINSTANCE__
         +0x000 unused           : 0n9460301
      +0x040 mSafeHooking     : 0n0
      +0x044 mNoImproveUnhook : 0n0
      +0x048 mValid           : 1
      +0x049 mNewHook         : 0
      +0x04a mDestroying      : 0
      +0x04b mIsWinsock2      : 0
      +0x04c mPatchExportTable : 0
      +0x050 mpHookedFunction : 0x00007ffc`7ee4e270 Void
      +0x058 mpCallbackFunction : 0x00007ffc`7f4d000e Void
      +0x060 mpShareCode      : (null) 
      +0x068 mpNextHook       : 0x00007ffc`7696df68  -> 0x00007ffc`7ee4e270 Void
      +0x070 mpTramp          : 0x00007ffc`7f500000 Void
      +0x078 mpHookStub       : 0x00007ffc`7f4d0000 tagIndirectAbsoluteJump
         +0x000 Opcode           : 0xff ''
         +0x001 modRm            : 0x25 'Unknown format characterUnknown format control character
         +0x002 Target           : 0
         +0x006 AbsoluteAddress  : 0x00007ffc`7f500000 Void
      +0x080 mpSparePage      : (null) 
   +0x228 pNextHook        : 0x00007ffc`7696df68  -> 0x00007ffc`7ee4e270 Void
   +0x230 pCallback        : 0x00007ffc`766d8990 Void
   +0x238 Flags            : 0
0:042> ln 0x00007ffc`766d8990
s:\6.1.0\discryptor\client service\modules\injectiondll\internetconnect.cpp(16)
(00007ffc`766d8990)   STGuard64!InternetConnectWHook   |  (00007ffc`766d8bf0)   STGuard64!InternetCloseHandleHookPostProcess
Exact matches:
    STGuard64!InternetConnectWHook (void *, wchar_t *, unsigned short, wchar_t *, wchar_t *, unsigned long, unsigned long, unsigned int64)
0:042> ln 0x00007ffc`7ee4e270
(00007ffc`7ee4e270)   wininet!InternetConnectW   |  (00007ffc`7ee4e4a0)   wininet!InternalInternetConnectA
Exact matches:
    wininet!InternetConnectW (void)
0:042> ln 0x00007ffc`7696df68 //our variable storing the next address shown before...
(00007ffc`7696df60)   STGuard64!InternetConnectWHooks+0x8   |  (00007ffc`7696df70)   STGuard64!InternetCloseHandleHooks
The interesting thing is that even though the hook is active, the callback points into the original adress instead of the MCHNextHookStub. How it could happen?

The next interesting thing is that even though the mCount in g_pHookCollection tels us that it has 0n40 items (indexed 0-39), but if i look at the index 40 (one item behind the array) I can find another entry of tagHookItem related to InternetConnectW hook.

Code: Select all

0:042> dt tagHookItem 0x00000000`0271eb00+0x240*0n40 -r
STGuard64!tagHookItem
   +0x000 hOwner           : 0x00007ffc`764e0000 HINSTANCE__
      +0x000 unused           : 0n9460301
   +0x008 hModule          : 0x00007ffc`7edd0000 HINSTANCE__
      +0x000 unused           : 0n9460301
   +0x010 ModuleName       : [260]  "wininet.dll"
   +0x114 ProcName         : [260]  "InternetConnectW"
   +0x218 pCode            : 0x00007ffc`7ee4e270 Void
   +0x220 pCodeHook        : 0x00000000`027288e0 CCodeHook
      +0x000 __VFN_table : 0x00007ffc`7670d0a0 
      +0x008 mpInUseArray     : (null) 
      +0x010 mLeakUnhook      : 0
      +0x018 mPatchAddr       : 0x00007ffc`7ee4e270 tagAbsoluteJmp
         +0x000 Opcode           : 0xff ''
         +0x001 modRm            : 0x25 'Unknown format characterUnknown format control character
         +0x002 Target           : 0x6b1dc0
      +0x020 mNewCode         : tagAbsoluteJmp
         +0x000 Opcode           : 0xff ''
         +0x001 modRm            : 0x25 'Unknown format characterUnknown format control character
         +0x002 Target           : 0x6b1dc0
      +0x026 mOldCode         : tagAbsoluteJmp
         +0x000 Opcode           : 0x48 'H'
         +0x001 modRm            : 0x8b ''
         +0x002 Target           : 0x588948c4
      +0x030 mpHookQueue      : (null) 
      +0x038 mHModule         : 0x00007ffc`7edd0000 HINSTANCE__
         +0x000 unused           : 0n9460301
      +0x040 mSafeHooking     : 0n0
      +0x044 mNoImproveUnhook : 0n0
      +0x048 mValid           : 1
      +0x049 mNewHook         : 1
      +0x04a mDestroying      : 1
      +0x04b mIsWinsock2      : 0
      +0x04c mPatchExportTable : 0
      +0x050 mpHookedFunction : 0x00007ffc`7ee4e270 Void
      +0x058 mpCallbackFunction : 0x00007ffc`7f4f000e Void
      +0x060 mpShareCode      : 0x00007ffc`00000000 Void
      +0x068 mpNextHook       : 0x00007ffc`7696df68  -> 0x00007ffc`7ee4e270 Void
      +0x070 mpTramp          : 0x00007ffc`7f500000 Void
      +0x078 mpHookStub       : (null) 
      +0x080 mpSparePage      : (null) 
   +0x228 pNextHook        : 0x00007ffc`7696df68  -> 0x00007ffc`7ee4e270 Void
   +0x230 pCallback        : 0x00007ffc`766d8990 Void
   +0x238 Flags            : 0
One would say that this entry is already cleared, but I can see that the data still looks valid. Maybe it is related to the problem. Maybe it is not.

What do you think can cause this?

PP
madshi
Site Admin
Posts: 10753
Joined: Sun Mar 21, 2004 5:25 pm

Re: Stack overflow crash

Post by madshi »

Hi again!

And sorry for the late reply...

The first "empty" item isn't really empty, I believe, it just doesn't have owner information/module information etc. It's most probably the internal LoadLibrary hook which then calls CheckHooks etc. You could double check what 0x00007ffc`85fe0000 points to. It's probably either the address where LoadLibraryExW calls LdrLoadDll. Or it could also be the entry point of LoadLibraryExW.

The first "out of range" item being a copy of the problematic item is interesting, but probably not meaningful. The list is (I think) maintained in such a way that if you delete an item of the HookCollection, the other items are moved (memcpy'ed) to the front of the array. So if at any point there were more items in the list than currently are, there are supposed to be some "dead" seemingly meaningful entries after the official "end" of the array. A simple memcpy should copy all that we need correctly, so the moving around shouldn't harm, as far as I can see.

> MCH did not modified the nexthook address or modified it in the wrong way.

Hmmmm... Do you pre-initialize the nexthook address with NULL or with the original API address? I'm wondering if madCodeHook really stored the wrong address there, or whether it just didn't change the value at all? That would be useful to know, can you check?

I'm wondering: After the changes we did the last time, I allowed a hook to be installed twice, if CheckHooks was run through by two different threads at the same time. If that happened, I detected that and removed one hook again. Could this situation maybe cause this problem somehow? I'm not sure...
madshi
Site Admin
Posts: 10753
Joined: Sun Mar 21, 2004 5:25 pm

Re: Stack overflow crash

Post by madshi »

P.S: Found the cause: If CheckHooks is called by multiple threads at once, the same API might be hooked multiple times, using the same "nextHook" variable. The "nextHook" is then overwritten multiple times, and after the duplicate hooks are removed later, the final "nextHook" value is no longer the correct one. I will fix this in the next build and let you know when it's available...
madshi
Site Admin
Posts: 10753
Joined: Sun Mar 21, 2004 5:25 pm

Re: Stack overflow crash

Post by madshi »

Should be fixed in this build:

http://madshi.net/madCollectionBeta.exe (installer 2.7.11.2)
EaSy
Posts: 150
Joined: Tue Oct 23, 2012 12:33 pm

Re: Stack overflow crash

Post by EaSy »

Great,
I updated MCH. I'll msg you if anything bad happens.

Sincerely
PP
Post Reply