Detoured function crashes on printf

Question

I'm made a DLL hook into an application.
Detoured a function like so:

typedef void (WINAPI *pUCPackets)(int a1, int a2, char* a3, int a4, int a5);
void WINAPI MyUCPackets(int a1, int a2, char* a3, int a4, int a5);
pUCPackets MyUC2Packets = (pUCPackets)(0x408050);

(...) some irrelevant code (...)

DetourTransactionBegin();
DetourUpdateThread(GetCurrentThread());
DetourAttach(&(PVOID&)MyUC2Packets, MyUCPackets);
if(DetourTransactionCommit() == NO_ERROR)
    cout << "[" << MyUCPackets << "] successfully detoured." << endl;

So then I tried to display the values inside the arguments on the detoured function by:

 void WINAPI MyUCPackets(int a1, int a2, char* a3, int a4, int a5)
 {
     printf( "%d ", a5 );
     printf("%d\n", a2);
     return MyUC2Packets(a1, a2, a3, a4, a5);
 }

But when ever the function gets called and I display the arguments, the app crashes.
But if I just leave the function like:

 void WINAPI MyUCPackets(int a1, int a2, char* a3, int a4, int a5)
 {
     //no prints whatsoever
     return MyUC2Packets(a1, a2, a3, a4, a5);
 }

It runs normally. Why is this happening?

OLLY coderipper:

Gate_00408050:                               ;<= Procedure Start

        MOV EDX,DWORD PTR SS:[ESP+0xC]
        PUSH EBP
        PUSH EDI
        MOV EDI,ECX
        XOR EBP,EBP
        MOV CL,BYTE PTR DS:[EDI+0x21C]
        TEST EDX,EDX
        JBE Gate_004080F0
        MOV EAX,DWORD PTR DS:[EDI+0x218]
        PUSH EBX
        PUSH ESI
        MOV DWORD PTR SS:[ESP+0x1C],EDX

Gate_00408074:

        MOV EDX,DWORD PTR SS:[ESP+0x14]
        DEC EAX
        TEST EAX,EAX
        MOV DL,BYTE PTR DS:[EDX]
        JLE Gate_004080A5
        LEA ESI,DWORD PTR DS:[EDI+EBP+0xEC7D]

Gate_00408086:

        MOV BL,BYTE PTR DS:[ESI+EAX]
        CMP BL,DL
        JA Gate_00408091
        SUB DL,BL
        JMP Gate_00408097

Gate_00408091:

        NOT BL
        INC BL
        ADD DL,BL

Gate_00408097:

        MOV BL,BYTE PTR DS:[ESI+EAX+0xFFFF8AD0]
        XOR DL,BL
        DEC EAX
        TEST EAX,EAX
        JG Gate_00408086

Gate_004080A5:

        MOV AL,BYTE PTR DS:[EDI+EBP+0xEC7D]
        CMP AL,DL
        JA Gate_004080B4
        SUB DL,AL
        JMP Gate_004080BA

Gate_004080B4:

        NOT AL
        INC AL
        ADD DL,AL

Gate_004080BA:

        MOV AL,BYTE PTR DS:[EDI+EBP+0x774D]
        MOV EBX,DWORD PTR SS:[ESP+0x14]
        XOR AL,DL
        MOV EDX,DWORD PTR SS:[ESP+0x18]
        XOR AL,CL
        MOV BYTE PTR DS:[EDX],AL
        XOR CL,AL
        MOV EAX,DWORD PTR DS:[EDI+0x218]
        ADD EBP,EAX
        INC EBX
        INC EDX
        MOV DWORD PTR SS:[ESP+0x14],EBX
        MOV DWORD PTR SS:[ESP+0x18],EDX
        MOV EDX,DWORD PTR SS:[ESP+0x1C]
        DEC EDX
        MOV DWORD PTR SS:[ESP+0x1C],EDX
        JNZ Gate_00408074
        POP ESI
        POP EBX

Gate_004080F0:

        POP EDI
        POP EBP
        RETN 0xC                             ;<= Procedure End

Answer 1

The signature of the MyUC2Packets is probably incorrect. Since the functions use stdcall calling convention they are required to clean up the stack before they return. If you call one of these functions with the wrong number of parameters the stack pointer will be incorrect when it returns.

The reason it does not happen when the print statements are removed is because the compiler is likely optimizing the forwarding call down to a single jmp instruction. When the print statements are included the detour function actually has work to do and adjusts the stack by an incorrect value before it returns. If MyUC2Packets expects 6 parameters but the function signatures only take 5 parameters this will cause problems any time the detour function can't be optimized down.

The code below demonstrates this by simulating the detour setup in your example. The function being hooked takes 4 parameters but the detour expects only 3. It simulates calls from a client expecting a function that takes 4 parameters.

#include <stdio.h>
#include <ios>
#pragma inline_depth(0)

typedef void (WINAPI *Function3)(int, int, int);
typedef void (WINAPI *Function4)(int, int, int, int);

void WINAPI FinalFunction(int x, int y, int z, int q);
void WINAPI DetourFunction(int x, int y, int z);
void WINAPI DetourFunctionPrint(int x, int y, int z);

Function3 callFinalFunction = reinterpret_cast<Function3>(FinalFunction);
Function4 callDetourFunction = reinterpret_cast<Function4>(DetourFunction);
Function4 callDetourFunctionPrint = reinterpret_cast<Function4>(DetourFunctionPrint);


void WINAPI FinalFunction(int x, int y, int z, int q)
{
    std::cout << x << " " << y << " " << z << " " << q << std::endl;
}

void WINAPI DetourFunction(int x, int y, int z)
{
    callFinalFunction(x, y, z); // Optimzed to a single jmp instruction.
}

void WINAPI DetourFunctionPrint(int x, int y, int z)
{
    printf("%d", x);
    printf("%d\n", y);
    callFinalFunction(x, y, z);
}


int main()
{
    // This works
    callDetourFunction(0, 1, 2, -1);

    // This does not
    callDetourFunctionPrint(0, 1, 2, -1);

    return 0;
}