Hooking VirtualProtect causes stack overflow

[take4-kait]

I'd like to inform you of a bug of mhook 2.4.

The bug causes stack overflow when hooking VirtualProtect and calling the original VirtualProtect from the hooked VirtualProtect, because VirtualProtect called from Mhook_SetHook function calls not the original VirtualProtect but the hooked VirtualProtect, therefore, the hooked VirtualProtect calls the original VirtualProtect recursively until stack overflow is occurred.

Here is simple sample code that causes stack overflow:

BOOL ( WINAPI RealVirtualProtect )( LPVOID lpAddress, SIZE_T dwSize, DWORD flNewProtect, PDWORD lpflOldProtect ) = (BOOL ( WINAPI )( LPVOID, SIZE_T, DWORD, PDWORD )) GetProcAddress( GetModuleHandleA( "kernel32.dll" ), "VirtualProtect" );

BOOL WINAPI HookVirtualProtect( LPVOID lpAddress, SIZE_T dwSize, DWORD flNewProtect, PDWORD lpflOldProtect )
{
return RealVirtualProtect( lpAddress, dwSize, flNewProtect, lpflOldProtect );
}

int main()
{
Mhook_SetHook( (PVOID*) &RealVirtualProtect, HookVirtualProtect );
return 0;
}

My workaround is to call the original VirtualProtect in Mhook_SetHook function.

Thank you in advance.

yes,it is,you can just use VirtualProtectEx instead of it.
And the same situation to RtlAllocateHeap...

What if you want to hook NtVirtualProtectMemory? You can't use any alternatives for that :/ Is there a solution for this?

[take4-kait]

Yes, I know not only VirtualProtect but also VirtualProtectEx and NtProtectVirtualMemory cause stack overflow.

My workaround is to use NtProtectVirtualMemory instead of VirtualProtect in Mhook_SetHook and Mhook_Unhook. And then, if we want to hook NtProtectVirtualMemory, the 3rd and the 4th VirtualProtect in Mhook_SetHook should be replaced with the code of the trampoline as follows:

if ( fnOrigNtProtectVirtualMemory == *ppSystemFunction ) fnNtProtectVirtualMemory = (_NtProtectVirtualMemory) (PVOID) pTrampoline->codeTrampoline;

DWORD NumberOfBytesToProtect = sizeof(MHOOKS_TRAMPOLINE);
PVOID BaseAddress = pTrampoline;
fnNtProtectVirtualMemory( GetCurrentProcess(), &BaseAddress, &NumberOfBytesToProtect, dwOldProtectTrampolineFunction, &dwOldProtectTrampolineFunction);

fnNtProtectVirtualMemory and fnOrigNtProtectVirtualMemory must be brought from ntdlld.dll by GetProcAddress in advance.

Thank you, I will test it :) Could you please have a look at my other issue, LdrLoadDll? Hooking that API causes crash on my application, however I can't see any reason to do that.

In the current case VirtualProtect hooking fails, because the "hooker" uses this API too. But if I'm right, it isn't using any LoadLibrary or similar functions.

Thank you for your answer :)

Can you provide pieces of your code about LdrLoadDll ? I have no sample of it.
And you should get all necessary module library before hook LdrLoadDll .

Argh, sorry, it was my faut. I didn't check my code enough and I made a small mistake.
LdrLoadDll is an undocumented function. if you're interested, you can find infos about it here:
http://undocumented.ntinternals.net/source/usermode/undocumented%20functions/executable%20images/ldrloaddll.html

Thanks for your kindness :)

[take4-kait]

I have tested hooking LdrLoadDll. But my code did not crash.
My sample code is below. (Mhook is the original one: I don't modify anything.)

#include <Windows.h>
#include "mhook-lib/mhook.h"

typedef struct _LSA_UNICODE_STRING {
  USHORT Length;
  USHORT MaximumLength;
  PWSTR  Buffer;
} LSA_UNICODE_STRING, *PLSA_UNICODE_STRING, UNICODE_STRING, *PUNICODE_STRING;

VOID (WINAPI* RtlInitUnicodeString)( _Inout_   PUNICODE_STRING DestinationString,  _In_opt_  PCWSTR SourceString ) = (VOID (WINAPI*)( PUNICODE_STRING, PCWSTR )) GetProcAddress( GetModuleHandleA( "ntdll.dll" ), "RtlInitUnicodeString" );

//=========================================================================
// Get the current (original) address to the functions to be hooked
//
NTSTATUS (NTAPI* LdrLoadDll) ( PWSTR SearchPath, PULONG DllCharacteristics, PUNICODE_STRING Name, PVOID* BaseAddress ) = (NTSTATUS (NTAPI*)( PWSTR, PULONG, PUNICODE_STRING, PVOID* )) GetProcAddress( GetModuleHandleA( "ntdll.dll" ), "LdrLoadDll" );
NTSTATUS (NTAPI* RealLdrLoadDll) ( PWSTR SearchPath, PULONG DllCharacteristics, PUNICODE_STRING Name, PVOID* BaseAddress ) = (NTSTATUS (NTAPI*)( PWSTR, PULONG, PUNICODE_STRING, PVOID* )) GetProcAddress( GetModuleHandleA( "ntdll.dll" ), "LdrLoadDll" );
NTSTATUS NTAPI HookLdrLoadDll( PWSTR SearchPath, PULONG DllCharacteristics, PUNICODE_STRING Name, PVOID* BaseAddress )
{
    OutputDebugString( Name->Buffer );
    return RealLdrLoadDll( SearchPath, DllCharacteristics, Name, BaseAddress );
}

//=========================================================================
// This is where the work gets done.
//
int WINAPI WinMain( HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine, int nShowCmd )
{
    // Set the hook
    if ( Mhook_SetHook( (PVOID*) &RealLdrLoadDll, HookLdrLoadDll ) ) {
        // Remove the hook
        UNICODE_STRING unicode_string;
        RtlInitUnicodeString( &unicode_string, L"A-dll.dll" );
        PVOID BaseAddress;
        LdrLoadDll( NULL, 0, &unicode_string, &BaseAddress );
        MessageBox( NULL, L"Pause...", L"Notification", MB_OK );

        Mhook_Unhook( (PVOID*) &RealLdrLoadDll );
    }

    return 0;
}

Note that this code needs DLL file (here A-dll.dll).

[Darker]

I came here from google. I get stack overflow error when calling original version of function as well. I documented everything on Stack Overflow: http://stackoverflow.com/q/28890309/607407

Is this a bug or is it simply impossible to call original function versions?

[0xshlomil]

I managed to hook VirtualProtect by using a flag that signals if the hooking engine is in the middle of the hooking process, exporting it from the lib and checking the flag from within my function hook. This prevents the recursion.