How to call original function after hotpatch

Question

I've written code to patch the "Sleep" function for example from Kernel32.dll. The patching works perfectly fine. The removal of the patch works perfectly fine. However, calling the original function does not work at all. It crashes badly.

#include <windows.h>
#include <iostream>

std::uint8_t* Patch(std::uint8_t* OrigFunc, std::uint8_t* HookFunc)
{
    DWORD dwProtect = 0;
    const static std::uint8_t jmp[] = {0xB8, 0x00, 0x00, 0x00, 0x00, 0xFF, 0xE0};
    const static std::int8_t jmp_size = sizeof(jmp) / sizeof(std::uint8_t);
    static std::uint8_t HookJump[jmp_size + 1] = {jmp_size};
    VirtualProtect(OrigFunc, jmp_size, PAGE_EXECUTE_READWRITE, &dwProtect);
    memcpy(&HookJump[1], OrigFunc, jmp_size);
    memcpy(OrigFunc, jmp, jmp_size);
    memcpy(OrigFunc + 1, &HookFunc, sizeof(void*));
    VirtualProtect(OrigFunc, jmp_size, dwProtect, &dwProtect);
    return HookJump;
}

void RemovePatch(std::uint8_t* OrigFunc, std::uint8_t* HookJump)
{
    DWORD dwProtect = 0;
    VirtualProtect(OrigFunc, HookJump[0], PAGE_EXECUTE_READWRITE, &dwProtect);
    memcpy(OrigFunc, &HookJump[1], HookJump[0]);
    VirtualProtect(OrigFunc, HookJump[0], dwProtect, &dwProtect);
}

typedef void (__stdcall *pSleep)(DWORD);
pSleep oSleep;

void __stdcall hSleep(DWORD MS)
{
    std::cout<<"HERE";
    oSleep(MS); //Crashes Here.
}

int main()
{
    std::uint8_t* OrigFunc = (std::uint8_t*)GetProcAddress(GetModuleHandle("kernel32.dll"), "Sleep");
    std::uint8_t* HookFunc = (std::uint8_t*)hSleep;
    std::uint8_t* HookJump = Patch(OrigFunc, HookFunc); //Works fine.

    oSleep = (pSleep)&HookJump[1];
    Sleep(1000);  //Prints Here then crashes immediately.

    RemovePatch(OrigFunc, HookJump); //Works fine.
    Sleep(1000); //Works fine.
}

Any ideas what my code is missing?

Answer 1

In the given code it appears that you store original bytes in a static array called HookJump, return a pointer to that array, then jump to the start of it as if it were valid machine code. It's not followed by the rest of the original function.

---

A better way to hook functions in Windows is to use Microsoft Detours.

Here's my (working sketch of a) Hook class, using Detours:

[Hook.h]

#pragma once
// Copyright (c) 2013 Alf P. Steinbach

#include <rfc/cppx/core/Non_copyable.h>     // cppx::Non_copyable
#include <rfc/cppx/core/macros/ASSERT.h>    // CPPX_ASSERT
#include <rfc/detours/Transaction.h>        // detours::Transaction

namespace detours {
    using cppx::Non_copyable;

    template< class Func >
    class Hook_
        : public Non_copyable
    {
    private:
        Func*   original_;
        Func*   replacement_;

    public:
        auto original_func() const
            -> Func*
        { return original_; }

        ~Hook_()
        { 
            if( original_ != nullptr )
            {
                Transaction().detach( original_, replacement_ ).commit();
            }
        }

        Hook_( Func* const original, Func* const replacement )
            : original_( original )
            , replacement_( replacement )
        {
            CPPX_ASSERT( original_ != nullptr );
            CPPX_ASSERT( replacement_ != nullptr );
            Transaction().attach( original_, replacement_ ).commit();
        }

        Hook_( Hook_&& other )
            : original_( other.original_ )
            , replacement_( other.replacement_ )
        { other.original_ = nullptr; other.replacement_ = nullptr; }
    };

    template< class Func >
    inline auto hook( Func* const original, Func* const replacement )
        -> Hook_<Func>
    { return Hook_<Func>( original, replacement ); }

}  // namespace detours

And here's the Transaction class that it uses, which in turn calls the Detours API:

[Transaction.h]

#pragma once
// Copyright (c) 2013 Alf P. Steinbach

#include <rfc/cppx/core/utility/If_.h>      // cppx::If
#include <rfc/cppx/core/Non_copyable.h>     // cppx::Non_copyable
#include <rfc/cppx/core/Type_.h>            // cppx::Type_

#include <thread>           // std::thread
#include <type_traits>      // std::is_function, std::enable_if

namespace detours {
    using cppx::If_;
    using cppx::Non_copyable;
    using cppx::Type_;
    using std::is_function;
    using std::thread;

    typedef thread::native_handle_type Thread_handle;

    class Basic_transaction
        : public Non_copyable
    {
    private:
        typedef Type_<void(*)()> Proc;

        bool    is_committed_;

        void raw_attach( Proc& original, Proc const replacement );
        void raw_detach( Proc& original, Proc const replacement );

    public:
        auto is_committed() const
            -> bool;
        void commit();

        auto update_thread( Thread_handle const h )
            -> Basic_transaction&;

        auto update_this_thread()
            -> Basic_transaction&;

        template< class Func, class Enabled = If_<is_function<Func>> >
        auto attach( Func*& original, Func* const replacement )
            -> Basic_transaction&
        {
            raw_attach(
                reinterpret_cast<Proc&>( original ),
                reinterpret_cast<Proc>( replacement )
                );
            return *this;
        }

        template< class Func, class Enabled = If_<is_function<Func>> >
        auto detach( Func*& original, Func* const replacement )
            -> Basic_transaction&
        {
            raw_detach(
                reinterpret_cast<Proc&>( original ),
                reinterpret_cast<Proc>( replacement )
                );
            return *this;
        }

        ~Basic_transaction();
        Basic_transaction();
    };

    class Transaction
        : public Basic_transaction
    {
    public:
        Transaction()
        { update_this_thread(); }
    };

}  // namespace detours

[Transaction.cpp]

#include "Transaction.h"

#include <rfc/cppx/core/throwing.h>
#include <rfc/cppx/core/macros/ASSERT.h>                    // CPPX_ASSERT
#include <rfc/detours_wrappers/detours_h.h>

using cppx::hopefully;
using cppx::fail;

typedef long Error_code;

namespace detours{

    auto Basic_transaction::is_committed() const
        -> bool
    { return is_committed_; }

    void Basic_transaction::commit()
    {
        CPPX_ASSERT( !is_committed_ );
        Error_code const code = ::DetourTransactionCommit();
        hopefully( code == 0 )
            || fail( "Basic_transaction::commit: DetourTransactionCommit failed", code );
        is_committed_ = true;
    }

    auto Basic_transaction::update_thread( Thread_handle const h )
        -> Basic_transaction&
    {
        Error_code const code = ::DetourUpdateThread( reinterpret_cast<HANDLE>( h ) );
        hopefully(code == 0)
            || fail("Transaction::update_thread: DetourUpdateThread failed", code);
        return *this;
    }

    auto Basic_transaction::update_this_thread()
        -> Basic_transaction&
    {
        return update_thread( Thread_handle( ::GetCurrentThread() ) );
    }

    void Basic_transaction::raw_attach( Proc& original, Proc const replacement )
    {
        Error_code const code = ::DetourAttach(
            reinterpret_cast<void**>( &original ),
            reinterpret_cast<void*>( replacement )
            );
        hopefully(code == 0)
            || fail("Transaction::attach: DetourAttach failed", code);
    }

    void Basic_transaction::raw_detach( Proc& original, Proc const replacement )
    {
        Error_code const code = ::DetourDetach(
            reinterpret_cast<void**>( &original ),
            reinterpret_cast<void*>( replacement )
            );
        hopefully(code == 0)
            || fail("Transaction::attach: DetourAttach failed", code);
    }

    Basic_transaction::~Basic_transaction()
    {
        if (!is_committed_)
        {
            Error_code const code = ::DetourTransactionAbort();
            hopefully( code == 0 )
                || fail( "Basic_transaction::<destroy>: DetourTransactionAbort failed", code );
        }
    }

    Basic_transaction::Basic_transaction()
        : is_committed_( false )
    {
        Error_code const code = ::DetourTransactionBegin();
        hopefully( code == 0 )
            || fail( "Basic_transaction::<init>: DetourTransactionBegin failed", code );
    }

}  // namespace detours

---

The Detours wrapper header:

[detours.h]

#pragma once
#include <rfc/winapi_wrappers/windows_h.h>
#include <microsoft_detours/detours.h>

I then use a cpp file to bring in a specific Detours implementation, e.g. for x86:

[detours_cpp.x86-32.cpp]

// Copyright (c) 2013 Alf P. Steinbach

#define DETOURS_INTERNAL                    // Necessary for DETOUR_TRACE
#include <rfc/detours_wrappers/detours_h.h>
#define DETOURS_X86
#define DETOURS_32BIT
#include <microsoft_detours/detours.cpp>    // Source
#include <microsoft_detours/disasm.cpp>     // More source, e.g. DetourCopyInstruction