2

im trying to create a sort of overlay of a game i play in order to display my ping in realtime as there has been a lot of issues and people blaming the ping so i just wanted an easy solution and add a realtime ping display in game :)

Anyways hooks has always been something that i have been strugling alot with, i get how it works but it just never works out for me, this is the code that i have seen in countless of places but after going through the trampoline it crashes:

void* detour::Hook(BYTE* src, BYTE* dst, int len) {
    BYTE *jmp = (BYTE*)malloc(len+5);
    DWORD dwback;

    VirtualProtect(src, len, PAGE_EXECUTE_READWRITE, &dwback);

    memcpy(jmp, src, len);
    jmp += len;

    jmp[0] = 0xE9;
    *(DWORD*)(jmp + 1) = (DWORD)(src + len - jmp) - 5;

    src[0] = 0xE9;
    *(DWORD*)(src+1) = (DWORD)(dst - src) - 5;

    VirtualProtect(src, len, dwback, &dwback);

    return (src + 6);
}

and i can see that it is really messy and its using malloc instead of VirtualAlloc etc.. but for some reason it works half the way :S anyways this was just something that i used in order to try and gain a better understanding of hooks, in the meantime i was working on some code of my own which is this:

DWORD detour::SetHook(DWORD src, DWORD dst, int len) {
    BYTE* backup;
    DWORD oldProtection;
    DWORD trampolineAddr;

    VirtualProtectEx(GetModuleHandle(NULL), (void*) dst, 5, PAGE_READWRITE, &oldProtection);

    //Create trampoline backup
    trampolineAddr = (DWORD) VirtualAllocEx(GetModuleHandle(NULL), NULL, 10, MEM_COMMIT |    MEM_RESERVE, PAGE_EXECUTE_READWRITE);
    memcpy((void*) trampolineAddr, (void*) dst, 5);
    memcpy((void*)(trampolineAddr + 5), (void*) 0xE9, 1);
    memcpy((void*)(trampolineAddr + 6), (void*) dst, 4);

    //set API hook
    memcpy((void*)dst, (void*) 0xE9, 1);
    memcpy((void*)(dst + 1), (void*)src, 4);

    VirtualProtectEx(GetModuleHandle(NULL), (void*) dst, 5, oldProtection, &oldProtection);

    return trampolineAddr;
}

and what this does is that it returns the address of where the trampoline has been created and i just make a jump to that address after the function that has replaced the hooked function has been called, allthought none of this works :/

so my questions in essense is really, why does the first hook only work half-way ? and what is so wrong about my hook ? it created the jump to the address, backs up the overwritten bytes and saves them in a trampoline which i jump to after my custom function but none of it works :(

would really appreciate some constructive feedback as this is something i have wanted to master for quite some time now :) Thanks in advance!

Paze
  • 191
  • 1
  • 13
  • Both bits of code changes the protection of the code to `read_write_execute` and then back to what it was originally. `malloc` is almost 100% certain to NOT give you executable memory, and you don't show what the second function uses for the memory allocation, so can't say, but seems like a plausible explanation as to why it would crash. Have you tried doing a disassembly of the trampoline? – Mats Petersson Nov 03 '14 at 21:54
  • What are the `memcpy(..., (void*)0xE9, 1)` statements supposed to do? Surely `0xE9` isn't always a valid address? – Cameron Nov 03 '14 at 22:03
  • Yes as i said the first code is not mine, its something that i used to gain a better understanding of it and it actually did work when compiled allthought it crashed after the trampoline, and for the memory allocation the second one im using VirtualAlloc, its in the code haha, and im just restoring the old protection once im done making modifications to it. i did attach a debugger to it and the hook is being placed succesfully, it runs my jump to function fine but when it is supposed to go back to the original function it somehow crashes because the address only contains garbage. – Paze Nov 03 '14 at 22:06
  • the 0xE9 means jump in assembly, so im placing a jump in the first byte of the function im hooking followed by 4 bytes that is the address of the new function. – Paze Nov 03 '14 at 22:07
  • @Paze: Aha, I see what the intent was -- you want to write `0xE9` (the opcode) to the buffer, but instead of setting it with e.g. `trampolineAddr[5] = 0xE9`, you're using `memcpy`. But `memcpy` reads bytes *from the address you give it*, not the address's value itself -- so it's not copying `0xE9` anywhere, rather it's trying to read a byte *at the address `0xE9`*, which contains garbage. The original code doesn't do that. – Cameron Nov 03 '14 at 22:10
  • Ahh okay i see now, but since trampolineAddr only contains the location of the allocated memory would that actually work ? doing trampolineAddr[5] = 0xE9; ? i would suspect something more like *trampolineAddr[5] = 0xE9; would be correct then ? allthought u gave me alot more to work with now :) thank you for your reply i will try and see if i can make something usefull out of it now. much appreciated. – Paze Nov 03 '14 at 22:18
  • @Paze: It sounds like you need to read up a bit more about pointers and raw memory manipulation :-) But neither your examples nor mine would compile, since `trampolineAddr` is a plain `DWORD` and not a pointer. I think it would be best if you cast the result of `VirtualAllocEx` to `BYTE*` (and changed the type of `trampolineAddr`) instead of using `DWORD`. Then you could set individual bytes with the `trampolineAddr[5] = 0xE9` notation, or the equivalent expression `*(trampolineAddr + 5) = 0xE9`. `*arr[offset]` doesn't make sense unless `arr` is an array of pointers. – Cameron Nov 03 '14 at 22:26
  • yeah i know, thats kind of why im diving down into this, and instead of just copy pasting some code that half work like the example i used im working on learning it all on my own, so im well aware and its a work in progress, but im definately improving :) – Paze Nov 03 '14 at 22:37

1 Answers1

-2

There are many problem with this code:

DWORD detour::SetHook(DWORD src, DWORD dst, int len) {

DWORD is not a type. You need to look it up but generally it will turn blue if it is a real type (int, float double are the basics).

BYTE* backup;

Also not a real type. Use an int or preferably a double if it is very big.

memcpy((void*) trampolineAddr, (void*) dst, 5);
memcpy((void*)(trampolineAddr + 5), (void*) 0xE9, 1);
memcpy((void*)(trampolineAddr + 6), (void*) dst, 4);

//set API hook
memcpy((void*)dst, (void*) 0xE9, 1);
memcpy((void*)(dst + 1), (void*)src, 4);

This is probably your problem right here, void * are bad practices and is very bad to have in your code. Good luck.

Ahmed

Ahmed
  • 9
  • 4
  • 2
    `DWORD` and `BYTE` most certainly are "real" types in Win32 programming. `DWORD` is an alias for `unsigned int`. And `void*` has several perfectly valid use cases. – Cameron Nov 03 '14 at 22:02
  • `BYTE` is usually an `unsigned char` and like @Cameron said it's valid types and heavily encouraged by microsoft if you look at their pages ^_^ for [example](http://msdn.microsoft.com/en-us/library/vstudio/9wsea8fy%28v=vs.100%29.aspx) – deW1 Nov 03 '14 at 22:03
  • yes these are valid types to use and in my oppinion is creates much cleaner and readable code, and i really don't think it would make a difference which one i would go with :) allthought thanks for your reply. – Paze Nov 03 '14 at 22:09