C++ Byte patch crashing game

[marhotiewer]

Hi there

Last week I've been trying to make a no damage hack by NOP'ing the call to the damage function. Simple right?
Well, not for me. After messing about with different signature scanners for a week I finally found one that worked for me.

When I figure this out I want to go ahead and make my own for the learning experience, and to figure out why the previous ones didn't work for me.

SO, I'm getting a bit off track here. I opened cheat cheat engine, found the call, and NOP'ed it. To be expected, it worked perfectly. My health wasn't dropping and even collisions and knockback was turned off. Time to make a hook in C++! I was ecstatic to finally start coding after weeks of playing with cheat engine, now having found a working sigscanner I could do just that!

    char bytes[] = "\xFF\x90\x00\x00\x00\x00\x83\xC4\x10\x8B\x47\x20\x8B\x40\x24\x83\xEC\x0C\x50\x8B\x00";
    char mask[] = "xx????xxxxxxxxxxxxxxx";
    DWORD address = FindPattern((DWORD)&bytes, mask);
    std::cout << "Damage function addr: " << address << std::endl;


    while (true)
    {
        if (GetAsyncKeyState(VK_HOME) & 1)
        {
            DWORD oldprotection, newprotection;
            for (size_t i = 0; i < 6; i++)
            {
                VirtualProtect((LPVOID)address, 1, PAGE_EXECUTE_READWRITE, &oldprotection);
                memset((void*)address, 0x90, 1);
                VirtualProtect((LPVOID)address, 1, oldprotection, &newprotection);


                std::cout << "NOP(0x90) > " << address << std::endl;
                address = address + 0x1;
            }
        }
        Sleep(100);
    }

0626FF5B   FF90 58030000    CALL DWORD PTR DS:[EAX+358] -- Sigscan returns address of this instruction
0626FF61   83C4 10          ADD ESP,10
0626FF64   8B47 20          MOV EAX,DWORD PTR DS:[EDI+20]
0626FF67   8B40 24          MOV EAX,DWORD PTR DS:[EAX+24]
0626FF6A   83EC 0C          SUB ESP,0C
0626FF6D   50               PUSH EAX
0626FF6E   8B00             MOV EAX,DWORD PTR DS:[EAX]

Unfortunately, I stumbled upon a problem. The hook seemed to work perfectly, but then after two minutes the game crashed. Access violation at 0x00000000. Okay, try again! Since then I've been studying my code closely and trying to find a pattern to make out what is making the game crash. Crashes seemed to occur whenever the game felt like it without any warning. At one point it gave me a different Access violation, on 0x00000008. This time visual studio let me see the assembly instruction where the crash occurred.

1002B572  mov         eax,dword ptr [ebx] {ebx:14846304}
1002B574  push        esi  
1002B575  mov         esi,dword ptr [eax+8] {eax:00000000} --CRASH
1002B578  push        edi  
1002B579  lea         edi,[esi+0DCh]

After trying for days I've been getting more lost by the minute. The crashes seemed to get even more random and I completely lost track of what is going on. I just cant figure out how such a simple byte batch as this could create such seemingly random crashes. I was hoping someone on this forum could help me figure this out! I'm all for learning so don't be afraid to let me think.

PS: While writing this unnecessary long story, I noticed I could use VirtualProtect only twice by setting the byte parameter to six bytes, and then setting it back when the memset operation is done. So I got that covered when I can be bothered :P

EDIT: I've played around a bit more, and I am starting to think the problem doesn't lie in my code but rather the assembly not liking what I'm doing.

[MikeRohsoft]

PS: While writing this unnecessary long story, I noticed I could use VirtualProtect only twice by setting the byte parameter to six bytes, and then setting it back when the memset operation is done. So I got that covered when I can be bothered :P

not only this, since u even use memset to nop them all

                VirtualProtect(reinterpret_cast<LPVOID>(address), 6, PAGE_EXECUTE_READWRITE, &oldprotection);
                memset(reinterpret_cast<void*>(address), 0x90, 6);
                VirtualProtect(reinterpret_cast<LPVOID>(address), 6, oldprotection, &newprotection);

no loop needed.

[marhotiewer]

not only this, since u even use memset to nop them all

                VirtualProtect(reinterpret_cast<LPVOID>(address), 6, PAGE_EXECUTE_READWRITE, &oldprotection);
                memset(reinterpret_cast<void*>(address), 0x90, 6);
                VirtualProtect(reinterpret_cast<LPVOID>(address), 6, oldprotection, &newprotection);

no loop needed.

Awesome! I took a closer look at the documentation of the function and I skipped over that.

[Hell_Demon]

The problem to me appears to be the stack cleanup after the call(i'm guessing it's a __cdecl calling convention) since the call is not executed you are most likely messing up the stack now, since the reservation of memory on the stack happens at the start of the callee

[marhotiewer]

The problem to me appears to be the stack cleanup after the call(i'm guessing it's a __cdecl calling convention) since the call is not executed you are most likely messing up the stack now, since the reservation of memory on the stack happens at the start of the callee

That is exactly what I was thinking. I started to look into it and tried NOP'ing a simple move call. Still the same seamingly random crashes.

I'm lost

[MikeRohsoft]

i thought your Edit says me that is kind of fixed xD
You may post me the full asm of the function and we can take a look if its a good idea to nop 6 bytes, maybe it's enough to switch a jmp to jnz somewhere

[Hidex73]

bool CustomWPM(HANDLE hProcess, LPVOID lpBaseAddress, byte* lpBuffer, char* mask)
{
	DWORD dwOldProtectionState;
	DWORD dummy;
	BOOL result = FALSE;
	DWORD Addr = (DWORD)lpBaseAddress;

	for (; *mask; mask++, lpBuffer++, Addr++)
	{
		if (*mask == 'x')
		{
			if (VirtualProtectEx(hProcess, (LPVOID)Addr, 1, PAGE_READWRITE, &dwOldProtectionState) == TRUE)
			{
				result = WriteProcessMemory(hProcess, (LPVOID)Addr, &*lpBuffer, 1, NULL);
				VirtualProtectEx(hProcess, (LPVOID)Addr, 1, dwOldProtectionState, &dummy);
			}
		}

	}
	return result;
}

bool DataCompare(byte* pData, byte* pattern, char* mask)
{
	for (; *mask; mask++, pattern++, pData++)
	{
		if (*mask == 'x' && *pData != *pattern)
			return false;
	}
	return true;
}

vector<DWORD> FindPattern(HANDLE hProcess, DWORD addr, DWORD length, byte* pattern, char* mask)
{
	MEMORY_BASIC_INFORMATION mbi = { 0 };
	DWORD offset = 0;
	DWORD dwTemp;
	vector<DWORD> Enderecos;

	while (offset < length)
	{
		VirtualQueryEx(hProcess, (LPCVOID)(addr + offset), &mbi, sizeof(MEMORY_BASIC_INFORMATION));
		if (mbi.Type == MEM_PRIVATE && (mbi.State != MEM_RESERVE))
		{
			byte* buffer = new byte[mbi.RegionSize];
			ReadProcessMemory(hProcess, mbi.BaseAddress, buffer, mbi.RegionSize, NULL);
			for (unsigned int i = 0; i < mbi.RegionSize; i++)
			{
				if (DataCompare(buffer + i, pattern, mask))
				{
					dwTemp = (DWORD)mbi.BaseAddress + i;
					Enderecos.push_back(dwTemp);
				}
			}
			delete[] buffer;
		}
		offset += mbi.RegionSize;
	}
	return Enderecos;
}

DWORD GetBaseAddr(DWORD id)
{
	MODULEINFO modinfo = { 0 };
	GetModuleInformation((HANDLE)id, GetModuleHandle(NULL), &modinfo, sizeof(MODULEINFO));
	return (DWORD)modinfo.lpBaseOfDll;
}

                                        Addresses = FindPattern(hProcess, BaseAddr, 0x7FFFFFFF, (PBYTE)"\x03\x3A\x00\x00\x00\x00\xD0\x00\x00\x00\x00\x28\x00\x00\x00\x00", "xx????x????x????");
					if (Addresses.size() != NULL)//AntiFix
					{
						DWORD Addr;
						for (unsigned int a = 0; a < Addresses.size(); a++)
						{
							Addr = Addresses[a];
							CustomWPM(hProcess, (LPVOID)Addr, (PBYTE)"\x2A\x3A\x00\x00\x00\x00\xD0\x00\x00\x00\x00\x28\x00\x00\x00\x00", "xx????x????x????");
						}
						Addresses.clear();
					}

[Mehodin1]

I suggest using memcopy for stuff like this,

Say you want to copy \x90\x90 into address 0x00400000
You would do:

std::memcpy((void*)0x00400000, "\x90\x90", 2);

Also, if it crashes the game, make sure you're allowed to WRITE to that address, (check through cheatengine)