Detours 3.0 application crashing when hooking SetRect

I am trying to hook SetRect function using Detours but my program is crashing directly after hooking. I tried hooking other functions such as DrawText before and everything worked fine and now I am not sure if there is a problem with my code or there is something about SetRect (and other similar functions) that I am not aware of causing the crash.

For testing I am using the provided Detours run with dll program:

withdll.exe -d:mydll.exe simpleprogram.exe

Where simpleprogram.exe is a simple one button C# application. Tried testing also on notepad, calc, firefox but all of them crashed too.

My hook DLL

#include <Windows.h>
#include <detours.h>
#include<string>
#include<fstream>
#include<iostream>
using namespace std;

wofstream out;

BOOL(__stdcall * T14)
(
_Out_ LPRECT lprc,
_In_  int    xLeft,
_In_  int    yTop,
_In_  int    xRight,
_In_  int    yBottom

) = SetRect;


__declspec(dllexport) BOOL  M14
(
_Out_ LPRECT lprc,
_In_  int    L,
_In_  int    T,
_In_  int    R,
_In_  int    B
){

    out << "SetRect: " << L << " " << T << " " << R << " " << B << endl;
    return T14(lprc, L, T, R, B);
}

BOOL WINAPI DllMain(HINSTANCE hinst, DWORD dwReason, LPVOID reserved)
{
    if (dwReason == DLL_PROCESS_ATTACH)
    {
        out.open("out.txt");
        out << "Attached" << endl;
        DetourRestoreAfterWith();
        DetourTransactionBegin();
        DetourUpdateThread(GetCurrentThread());
        DetourAttach(&(PVOID&)T14, M14);
        DetourTransactionCommit();
    }
    else if (dwReason == DLL_PROCESS_DETACH)
    {
        out << "detached" << endl;
        out.close();
        DetourTransactionBegin();
        DetourUpdateThread(GetCurrentThread());
        DetourDetach(&(PVOID&)T14, M14);
        DetourTransactionCommit();
    }

    return TRUE;
}

The output I am getting:

Attached

SetRect: 138 161 323 161

So the program is crashing after the first SetRect function is called, any ideas why this is happening ?

Solution

Your detoured function's calling convention is wrong.

Visual C++ projects default to __cdecl, but ALL of the Windows API uses WINAPI (__stdcall).

You are trashing the callstack due to differences in calling convention. This issue does not exist in 64-bit software since there is only one calling convention, but for x86 it is critically important that you match any calling convention defined in the original function's prototype.

__declspec(dllexport) BOOL WINAPI M14
(
_Out_ LPRECT lprc,
_In_  int    L,
_In_  int    T,
_In_  int    R,
_In_  int    B
){

    out << "SetRect: " << L << " " << T << " " << R << " " << B << endl;
    return T14(lprc, L, T, R, B);
}

On another, possibly even more serious and difficult to debug note. I would refrain from doing C++ stream I/O in DllMain (...). Unless you statically linked your DLL to the MSVCRT this brings in dependencies on other DLLs while you are holding the loader lock.

You can wind up deadlocking if you call any function from DllMain that is not statically linked or part of kernel32.dll or user32.dll. The good news is kernel32 and user32 have a plethora of string functions -- enough that you should not even need the C++ standard library ;)