microsoft / Detours

Detours is a software package for monitoring and instrumenting API calls on Windows. It is distributed in source code form.
MIT License
5.01k stars 979 forks source link

Exception upon hooking the API GetKeyboardState #250

Closed derricck closed 1 year ago

derricck commented 1 year ago

I'm trying to hook the WINAPI GetKeyboardState but the process throws an exception as soon I type something:

detours

Commenting on these lines I still get an exception:

//lpKeyState[VK_CONTROL] = 0;
//lpKeyState[VK_LCONTROL] = 0;

I have tested hooking other APIs and they did work 'correctly'. I'm doing something wrong?

    #include "pch.h"
    #include "detours.h"
    #include "dllmain.h"

    BOOL(WINAPI* GetKeyboardState_Hook)(PBYTE) = GetKeyboardState;

    BOOL WINAPI HookedGetKeyboardState(PBYTE& lpKeyState) {

        if (lpKeyState == NULL)
            return 1;

        //lpKeyState[VK_CONTROL] = 0;
        //lpKeyState[VK_LCONTROL] = 0;

        return GetKeyboardState(lpKeyState);
    }

    BOOL APIENTRY DllMain(HMODULE hModule, DWORD  ul_reason_for_call, LPVOID lpReserved)
    {
        switch (ul_reason_for_call)
        {
        case DLL_PROCESS_ATTACH:
        {
            Sleep(1000);
            DetourTransactionBegin();
            DetourUpdateThread(GetCurrentThread());
            DetourAttach(&(PVOID&)GetKeyboardState_Hook, HookedGetKeyboardState);

            LONG lError = DetourTransactionCommit();
            if (lError != NO_ERROR) {
                MessageBox(HWND_DESKTOP, L"Failed to detour", L"", MB_OK);
                return FALSE;
            }
        }
        break;

        case DLL_PROCESS_DETACH:
        {
            DetourTransactionBegin();
            DetourUpdateThread(GetCurrentThread());
            DetourDetach(&(PVOID&)GetKeyboardState_Hook, HookedGetKeyboardState);

            LONG lError = DetourTransactionCommit();
            if (lError != NO_ERROR) {
                MessageBox(HWND_DESKTOP, L"DLL_PROCESS_DETACH", L"", MB_OK);
                return FALSE;
            }
        }
        break;
        }

        return TRUE;
    }
LeoDavidson commented 1 year ago

Your hook has a different signature to the real API. It's taking a PBYTE& argument instead of PBYTE.

sylveon commented 1 year ago

Also, I believe you're stack overflowing due to calling GetKeyboardState within your hook function (which calls your hook function recursively). You should be calling GetKeyboardState_Hook (which is set to be a trampoline to the real function while you have the hook active)

derricck commented 1 year ago

Thank you guys, the problem was exactly both things mentioned!

derricck commented 1 year ago
BOOL (WINAPI* GetKeyboardState_Hook)(PBYTE) = GetKeyboardState;

BOOL WINAPI HookedGetKeyboardState(PBYTE lpKeyState) {
    //...
}

@sylveon I put a breakpoint inside of the hooked function but lpKeyState return 0'\0'.

devenv_2022-08-20_21-09-57

When I call GetKeyboardState without hooking the API, it returns an array: image

My goal is to modify the key down state from some keys I'm doing something wrong?

sylveon commented 1 year ago

That's just the IDE not being able to differentiate between a pointer to a single byte, or a pointer to an array of bytes.

derricck commented 1 year ago

Do you what would be the 'proper' way to return a modified value in this API?

BOOL(WINAPI* GetKeyboardState_Hook)(PBYTE) = GetKeyboardState;

BOOL WINAPI HookedGetKeyboardState(PBYTE lpKeyState) 
{
    for (int i = 1; i < sizeof(lpKeyState); i++)
        lpKeyState[i] = { '\0' };

    //lpKeyState[VK_SHIFT] |= 0x80;
       lpKeyState[VK_SHIFT] = 1;

    SetKeyboardState((LPBYTE)lpKeyState);

    return GetKeyboardState_Hook(lpKeyState);
}

BOOL(WINAPI* SetKeyboardState_Hook)(LPBYTE) = SetKeyboardState;

BOOL WINAPI HookedSetKeyboardState(LPBYTE lpKeyState)
{
    for (int i = 1; i < sizeof(lpKeyState); i++)
        lpKeyState[i] = { '\0' };

       //lpKeyState[VK_SHIFT] |= 0x80;
       lpKeyState[VK_SHIFT] = 1;

    return SetKeyboardState_Hook(lpKeyState);
}

When i call GetKeyboardState some values of the array are with weird characters:

        BYTE KeybStateCur[256];
        GetKeyboardState((LPBYTE)&KeybStateCur);

VK_Shift is 16, its correct '1' but instead of shift be down, when i type something looks like its 'forcing' the alt to be down devenv_2022-08-20_22-25-10