How to correctly use VirtualFreeEx when trying to inject a dll

Question

I'm trying to make a DLL injector but there are parameter errors in the functions.

I've tried changing the variable to char*, I've confirmed that the process ID is correct, I can't compile it in x64 or x86 because I'm using code blocks but the program I'm trying it on was created with code blocks, the DLL works with other injectors, I've confirmed that the dll path is correct.

void EchoLastError()
{
    std::string ToExecute;
    std::stringstream Command;
    Command << "echo " << GetLastError();
    ToExecute = Command.str();
    system(ToExecute.c_str());
    Command.str(std::string());
}
BOOL CreateRemoteThreadInject(DWORD IDofproc, const char * dll)
{
    HANDLE Process;
    LPVOID Memory, LoadLibrary;
    if (!IDofproc)
    {
        return false;
    }
    Process = OpenProcess(PROCESS_CREATE_THREAD | PROCESS_QUERY_INFORMATION | PROCESS_VM_READ | PROCESS_VM_WRITE | PROCESS_VM_OPERATION, FALSE, IDofproc);

    EchoLastError();

    system("pause");
    LoadLibrary = (LPVOID)GetProcAddress(GetModuleHandle("kernel32.dll"), "LoadLibraryA");

    Memory = (LPVOID)VirtualAllocEx(Process, NULL, strlen(dll) + 1, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE);

    WriteProcessMemory(Process, (LPVOID)Memory, dll, strlen(dll) + 1, NULL);

    CreateRemoteThread(Process, NULL, NULL, (LPTHREAD_START_ROUTINE)LoadLibrary, (LPVOID)Memory, NULL, NULL);
    EchoLastError();
    CloseHandle(Process);

    VirtualFreeEx(Process, (LPVOID)Memory, 0, MEM_RELEASE);
    EchoLastError();
    MessageBox(NULL, "Injected", "", MB_OK);

    return true;
}



//other
bool Injectstuff(DWORD processId, char* dllpath)
{
    std::stringstream kds;
    kds << "echo Process ID: " << processId;
    std::string dl = kds.str();
    system(dl.c_str());
    EchoLastError();
    HANDLE hTargetProcess = OpenProcess(PROCESS_ALL_ACCESS, false, processId);
    EchoLastError();
    system("pause");
    if (hTargetProcess)
    {
        LPVOID LoadLibAddress = (LPVOID)GetProcAddress(GetModuleHandleA("kernel32.dll"), "LoadLibraryA");
        EchoLastError();
    system("pause");
        LPVOID LoadPath = VirtualAllocEx(hTargetProcess, 0, strlen(dllpath), MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE);
        EchoLastError();
    system("pause");
        HANDLE RemoteThread = CreateRemoteThread(hTargetProcess, 0, 0, (LPTHREAD_START_ROUTINE)LoadLibAddress, LoadPath, 0, 0);
        EchoLastError();
    system("pause");
        WaitForSingleObject(RemoteThread, INFINITE);
        EchoLastError();
    system("pause");
        VirtualFreeEx(hTargetProcess, LoadPath, strlen(dllpath), MEM_RELEASE);
        EchoLastError();
    system("pause");
        CloseHandle(RemoteThread);
        CloseHandle(hTargetProcess);
        return true;
    }
    return false;
}

The first function prints 87 on VirtualFreeEx, the second prints 6 on VirtualFreeEx. How do I fix these? I'm using GNU GCC compiler.

Answer 1

You are not doing any error handling at all. OpenProcess(), VirtualAllocEx(), WriteProcessMemory(), CreateRemoteThread(), all these function can fail, you need to handle that. And GetLastError() only has meaning if they actually do fail.

In CreateRemoteThreadInject(), if CreateRemoteThread() succeeds, you are not waiting for the thread to complete before attempting to free the allocated memory. And you are closing the HANDLE to the process before using that HANDLE to free the allocated memory. And you are not closing the HANDLE returned by CreateRemoteThread().

You are doing things in the right order in Injectstuff(), but you are still lacking adequate error handling, and also you are not allocating enough memory for a null terminator on the DLL path string.

But, why do you have two functions doing essentially the same thing? The only real difference between them is Injectstuff() is asking OpenProcess() for more permissions than it really needs, whereas CreateRemoteThreadInject() asks for just the specific permissions it actually needs.

On a side note, your use of system() to execute echo commands is completely useless. You should just write to std::cout or std::cerr instead, and flush it if needed. There is no need to shell out a system command process to execute its echo command at all.

Try something more like this instead:

void DisplayLastError(const char *operation, int err)
{
    std::cerr << "Error ";
    if (err) std::cerr << err << " ";
    std::cerr << operation << std::endl;
}

void DisplayLastError(const char *operation)
{
    DisplayLastError(operation, GetLastError());
}

bool CreateRemoteThreadInject(DWORD IDofproc, const char * dll)
{
    if (!IDofproc)
        return false;

    LPVOID pLoadLibrary = (LPVOID) GetProcAddress(GetModuleHandle(TEXT("kernel32.dll")), "LoadLibraryA");
    if (!pLoadLibrary)
    {
        DisplayLastError("getting LoadLibrary pointer");
        return false;
    }

    HANDLE hProcess = OpenProcess(PROCESS_CREATE_THREAD | PROCESS_QUERY_INFORMATION | PROCESS_VM_READ | PROCESS_VM_WRITE | PROCESS_VM_OPERATION, FALSE, IDofproc);
    if (!hProcess)
    {
        DisplayLastError("opening the process");
        return false;
    }

    LPVOID pMemory = VirtualAllocEx(hProcess, NULL, strlen(dll) + 1, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE);
    if (!pMemory)
    {
        DisplayLastError("allocating memory");
        CloseHandle(hProcess);
        return false;
    }

    if (!WriteProcessMemory(hProcess, pMemory, dll, strlen(dll) + 1, NULL))
    {
        DisplayLastError("writing to allocated memory");
        VirtualFreeEx(hProcess, pMemory, 0, MEM_RELEASE);
        CloseHandle(hProcess);
        return false;
    }

    HANDLE hThread = CreateRemoteThread(hProcess, NULL, 0, (LPTHREAD_START_ROUTINE)pLoadLibrary, pMemory, 0, NULL);
    if (!hThread)
    {
        DisplayLastError("creating remote thread");
        VirtualFreeEx(hProcess, pMemory, 0, MEM_RELEASE);
        CloseHandle(hProcess);
        return false;
    }

    WaitForSingleObject(hThread, INFINITE);

    DWORD dwExitCode = 0;
    GetExitCodeThread(hThread, &dwExitCode);

    CloseHandle(hThread);

    VirtualFreeEx(hProcess, pMemory, 0, MEM_RELEASE);
    CloseHandle(hProcess);

    if (!dwExitCode)
    {
        DisplayLastError("loading dll", 0);
        return false;
    }

    MessageBox(NULL, TEXT("Injected"), TEXT(""), MB_OK);
    return true;
}

bool Injectstuff(DWORD processId, char* dllpath)
{
    std::cout << "Process ID: " << processId << std::endl;
    return CreateRemoteThreadInject(processId, dllpath);
}

Also, note that the code needed to detect if LoadLibraryA() is successful or not works properly only when the target process is 32-bit. The function passed to CreateRemoteThread() must always returns a 32-bit DWORD, and LoadLibraryA() returns a 32-bit HMODULE when called in a 32-bit process, but it returns a 64-bit HMODULE when calling in a 64-bit process. A thread cannot return a 64-bit exit code, and GetExitCodeThread() cannot retrieve a 64-bit exit code, so the returned HMODULE will get truncated, which can lead to inaccurate results. So, it is not really appropriate to use LoadLibraryA() directly as the thread function when injecting into a 64-bit process, unless you don't care about the result of the load. If you need that, you can instead inject a small function thunk that indirectly calls LoadLibrary() and saves the result to a memory address that the injector can then read from using ReadProcessMemory() when the thread is finished. Or use a different injection technique.