Intercepting game function with C and Assembler, game unstable with minimum changes

Question

I am reverse engeneering a really old game (Mu 97d) for the sake of adding new things to the game.
What I am doing is intercepting the game with a DLL, and in the DLL do the calculations so I can add the new things.

I have this code that intercepts an ‘if’ statement, and I redirect it to my own function – based on the input – my function returns true or false.

C code:

/* Called on player load - 12 bytes */
*(BYTE*)(0x004798CD  + 0) = 0x50;  /* PUSH EAX */
*(BYTE*)(0x004798CD  + 1) = 0xE8;  /* CALL */
*(DWORD*)(0x004798CD + 2) = (DWORD)&Unk11 - (0x004798CD + 6); /* FUNCTION TO CALL */
*(BYTE*)(0x004798CD  + 6) = 0x85;  /* TEST */
*(BYTE*)(0x004798CD  + 7) = 0xC0;  /* EAX, EAX */
*(BYTE*)(0x004798CD  + 8) = 0x58;  /* POP EAX */
*(BYTE*)(0x004798CD  + 9) = 0x74;  /* JE */
*(BYTE*)(0x004798CD  + 10) = 0x15;  /* 15 */
*(BYTE*)(0x004798CD  + 11) = 0x90;  /* NOP */

ASM part that I am intercepting

[...]
004798A1  |.  81E7 FF000000 AND EDI,000000FF
004798A7  |.  8D0440        LEA EAX,[EAX*2+EAX]
    004798AA  |.  C1E0 02       SHL EAX,2
004798AD  |.  99            CDQ
004798AE  |.  F7FF          IDIV EDI
004798B0  |.  8BD8          MOV EBX,EAX
004798B2  |.  B8 67666666   MOV EAX,66666667
004798B7  |.  F7EF          IMUL EDI
004798B9  |.  D1FA          SAR EDX,1
004798BB  |.  8BC2          MOV EAX,EDX
004798BD      03D3          ADD EDX,EBX
004798BF      C1E8 1F       SHR EAX,1F
004798C2      8D5410 04     LEA EDX,[EDX+EAX+4]
004798C6      66:0156 12    ADD WORD PTR DS:[ESI+12],DX
004798CA      66:8B06       MOV AX,WORD PTR DS:[ESI]
004798CD      66:3D 8301    CMP AX,183 ]---------------------- FROM HERE
004798D1      7C 1A         JL SHORT 004798ED
004798D3      66:3D 8601    CMP AX,186
004798D7      7F 14         JG SHORT 004798ED ]--------------- TO HERE
004798D9      83F9 09       CMP ECX,9
004798DC      B8 09000000   MOV EAX,9
004798E1      7F 02         JG SHORT 004798E5
004798E3      8BC1          MOV EAX,ECX
004798E5      03C0          ADD EAX,EAX
004798E7      66:0146 12    ADD WORD PTR DS:[ESI+12],AX
004798EB      EB 13         JMP SHORT 00479900
004798ED      83F9 09       CMP ECX,9
004798F0  |.  B8 09000000   MOV EAX,9
004798F5  |.  7F 02         JG SHORT 004798F9
004798F7  |.  8BC1          MOV EAX,ECX
004798F9  |>  8D1440        LEA EDX,[EAX*2+EAX]
004798FC  |.  66:0156 12    ADD WORD PTR DS:[ESI+12],DX
00479900  |>  8D51 F7       LEA EDX,[ECX-9]
00479903  |.  33C0          XOR EAX,EAX
00479905  |.  85D2          TEST EDX,EDX
00479907  |.  7E 1D         JLE SHORT 00479926
00479909  |.  BB 04000000   MOV EBX,4
0047990E  |.  BF 05000000   MOV EDI,5
00479913  |>  85C0          /TEST EAX,EAX
00479915  |.  75 06         |JNE SHORT 0047991D
00479917  |.  66:015E 12    |ADD WORD PTR DS:[ESI+12],BX
0047991B  |.  EB 04         |JMP SHORT 00479921
0047991D  |>  66:017E 12    |ADD WORD PTR DS:[ESI+12],DI
00479921  |>  40            |INC EAX
[...]

This is the code from my DLL, the function right now behaves as the game would do.

BOOL Unk11(short int a1)
{
    if((a1 >= 387 && a1 <= 390))
        return TRUE;
    return FALSE;
}

But, if I add a second condition to the IF:

BOOL Unk11(short int a1)
{
    if((a1 >= 387 && a1 <= 390) || (a1 >= 404 && a1 <= 415))
        return TRUE;
    return FALSE;
}

The game becomes unstable, even with a simple debug function that writes to a log file, the game becomes unstable, example:

BOOL Unk11(short int a1)
{
    Debug("Hello");
    if((a1 >= 387 && a1 <= 390))
        return TRUE;
    return FALSE;
}

With Ollydbg I executed the game, and checked the ASM part of my function, and this is what I’ve got:

CPU Disasm
Address   Hex dump          Command                                  Comments
7C721B30    55              PUSH EBP
7C721B31    8BEC            MOV EBP,ESP
7C721B33    8B45 08         MOV EAX,DWORD PTR SS:[EBP+8]
7C721B36    05 7DFEFFFF     ADD EAX,-183
7C721B3B    66:B9 0300      MOV CX,3
7C721B3F    66:3BC8         CMP CX,AX
7C721B42    1BC0            SBB EAX,EAX
7C721B44    40              INC EAX
7C721B45    5D              POP EBP
7C721B46    C3              RETN

Why the ADD EAX is -183? Shouldn’t it be 183? Where is the ‘<= 390’ in this code?
I tried with different calling conventions, but I get the same behaviour – game unstable.

Why is the game unstable even with a minimum game? The game is old, 2003 or so – could it be a compatibility issue with my DLL? I am using Visual Studio 2010.

I hope this is enough info, if not, say so!
Thanks!

asm c++

Ian Cook · Answer

To add to Paweł Łukasik's answer, the issue with ECX surrounds what is known as a 'calling convention'.
Calling conventions set out the obligations on both the caller of a function and on the called function itself.
Microsoft's 32-bit calling convention is described here.
It says that an obligation on the called function is -

The compiler generates prolog and epilog code to save and restore the
ESI, EDI, EBX, and EBP registers, if they are used in the function.

Conversely, this implies that the caller of a function must expect the values in other registers to NOT survive the call. (This is most obviously the case in respect of EAX - often used for the return value.)
The compiler of your Unk11 function is doing the right thing in that, since it is modifying EBP, it is preserving it's value.  It's under no obligation though to preserve ECX.
For the other side of the contact, there is no compiler here calling the function though. Instead it's your hand-crafted binary code doing the calling, so the obligation is on you to preserve ECX at that end.

Fixing It
Obviously one solution is using inline assembler as you have done in your answer.  Here you are modifying ESI and so are correctly preserving it.
However, as you have discovered, if you call another function it is up to you, in your hand-crafted assembly, to abide by the calling convention.  It is almost certain that Debug is modifying ECX or EDX (as it is allowed to do) giving the adverse effect you are seeing.
So, if you also preserve ECX and EDX in your assembly function I suspect that you will find a call to Debug will work (or at least, if it fails, it's for some other reason.)
Something else to consider is that if you were to need any more complex logic in your DLL function, using assembly can be a pain.  Instead you might want to consider writing the function using normal C, as you were originally doing, but writing a very simple assembly wrapper that preserves the values of these additional registers around a call to your easier-to-code C function.

Feche1320 · Answer

Okay, so after a few hours this is what I did - instead of a C style function, I made it with ASM; this is the result:
Naked (Unk11)
{
    _asm
    {
        PUSH ESI
        MOV ESI, DWORD PTR SS:[ESP+8]

CMP ESI, 0x183 //387
        JL False
        CMP ESI, 0x186 //390
        JLE True
False:
        MOV EAX, FALSE
        JMP Exit
True:   
        MOV EAX, TRUE
Exit:
        POP ESI
        RETN
    }
}

Now this is working correctly, the only thing that I still can't figure it out, is why if I add the 'Debug();' function, the game starts to behave strange.
For example, my armor goes from 95 to 45 by just adding that function.

Paweł Łukasik · Answer

It's not add -183. 0xFFFFFE7d is equal to -387. The original conditions is this
387 <= x <= 390

if we add -387 to all the sides we get
0 <= x + (-387) <= 3

and this is exactly what's checked in the code.
As why the code is unstable? Well it would require a bit more detailed analysis (debugging?) but what I can see is this. Just after the code you are replacing, ECX is used (see 004798D9 83F9 09 CMP ECX,9) and yet your function destroy the value of ECX (see 7C721B3B 66:B9 0300 MOV CX,3). Try saving this value and see if it become more stable.

Intercepting game function with C and Assembler, game unstable with minimum changes

3 Answers

Add your own answers!

Ask a Question