# C++ DLL Injector

I made a DLL Injector in C++. It provides multiple functionalities. What could be added or improved in the code? It works well as far as I've used it.

#include <iostream>
#include <Windows.h>
#include <TlHelp32.h>

void display_usage(const char* argv0_)
{
std::cout << "Usage: " << argv0_ << " <DLL File> <[-c <Program Path To Execute>] | [-p <Process Name>] | [-w <Window Title>]>" << std::endl;
}

DWORD get_process_id_by_creation(char* program_path_, HANDLE* creation_handle_)
{
// initalize variables
STARTUPINFO startup_information;
PROCESS_INFORMATION process_information;
memset(&startup_information, 0, sizeof(startup_information));
memset(&process_information, 0, sizeof(process_information));

startup_information.cb = sizeof(startup_information);

// create process
if (CreateProcess(NULL, program_path_, 0, 0, false, CREATE_SUSPENDED, 0, 0, &startup_information, &process_information))
{
return process_information.dwProcessId;
}

return NULL;
}

DWORD get_process_id_by_process_name(const char* process_name_)
{
PROCESSENTRY32 process_entry = {sizeof(PROCESSENTRY32)};
HANDLE processes_snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);

// loop through all process to find one that matches the process_name_
if (Process32First(processes_snapshot, &process_entry))
{
do
{
if (strcmp(process_entry.szExeFile, process_name_) == 0)
{
CloseHandle(processes_snapshot);
return process_entry.th32ProcessID;
}
} while (Process32Next(processes_snapshot, &process_entry));
}

CloseHandle(processes_snapshot);
return NULL;
}

DWORD get_process_id_by_window_title(const char* window_title_)
{
// get a handle to window using the window name
HWND window_handle = FindWindow(NULL, window_title_);
if (window_handle == NULL)
{
return NULL;
}

// return the process id of the window handle we found
DWORD process_id;
return process_id;
}

bool inject_dll(DWORD process_id_, const char* dll_file_)
{
// get the full path of the dll file
TCHAR full_dll_path[MAX_PATH];
GetFullPathName(dll_file_, MAX_PATH, full_dll_path, NULL);

{
return false;
}

// open the process
HANDLE process_handle = OpenProcess(PROCESS_ALL_ACCESS, false, process_id_);
if (process_handle == NULL)
{
return false;
}

// allocate space to write the dll location
{
CloseHandle(process_handle);
return false;
}

// write the dll location to the space we previously allocated
BOOL wrote_memory = WriteProcessMemory(process_handle, dll_parameter_address, full_dll_path, strlen(full_dll_path), NULL);
if (wrote_memory == false)
{
CloseHandle(process_handle);
return false;
}

// launch the dll using LoadLibraryA
{
CloseHandle(process_handle);
return false;
}

CloseHandle(process_handle);
return true;
}

int main(int argc_, char** argv_)
{
// ensure proper usage
if (argc_ != 4 || strlen(argv_[2]) != 2)
{
display_usage(argv_[0]);
return -1;
}

// obtain the process ID
DWORD process_id = NULL;
HANDLE creation_handle = NULL;
switch (argv_[2][1])
{
case 'C':
case 'c':
process_id = get_process_id_by_creation(argv_[3], &creation_handle);
break;
case 'P':
case 'p':
process_id = get_process_id_by_process_name(argv_[3]);
break;
case 'W':
case 'w':
process_id = get_process_id_by_window_title(argv_[3]);
break;
default:
display_usage(argv_[0]);
return -2;
}

// inject the dll
std::cout << "Obtained Process ID: " << process_id << std::endl;
std::cout << "Injection: " << (inject_dll(process_id, argv_[1]) ? "Success" : "Failure") << std::endl;

// if we created the process, resume it
if (creation_handle)
{
}

return 0;
}


• At first sight this code (excluding std::cout) is C, not C++. Do you want a C++ review (which will obviously say that you have to write C++ish code) or a C review? Nov 22, 2017 at 14:01
• Hey, I'd like a C++ review - what would I change to make this more C++ like and less C? Nov 25, 2017 at 22:33

## Don't mix logging/debugging with actual program logic

Consider the following line:

std::cout << "Injection: " << (inject_dll(process_id, argv_[1]) ? "Success" : "Failure") << std::endl;


If you keep iterating on this project, sooner or later you'll have a #define LOGGING_ENABLED, or a verbosity control argument. It would then be too easy to accidentally disable a piece of program logic.

At the end of the day, injecting the dll and logging wether the injection is successfull is too separate concerns. Keep your concerns separated.

## inject_dll()'s error handling.

The function as written is a textbook example of why some people argue that functions should only have a single return statement after initial sanity-checks. Even if this function is correct, that coding style of manually cleaning the proper stuff in each exit branch of the function is bound to bite you in the ass sooner or later and should be avoided in general.

The best way to fix this would be to use RAII, either using a scope_exit() type function, or a std::unique_ptr<> with a custom deleter.

## Don't do massive allocations on the stack

The following is too big to put in the stack in my opinion:

TCHAR full_dll_path[MAX_PATH];


std::vector<TCHAR> full_dll_path(MAX_PATH);

• You do know MAX_PATH is defined to 260 by the Win32 API headers? So I doubt this classifies as a massive allocation... Nov 22, 2017 at 14:50
• @hoffmale Yes, I guess my embedded bias is showing :P.
– user128454
Nov 22, 2017 at 15:20

# Compilation flags

Please specify for which sets of compilation flags this code is expected to work (it will break if compiled with UNICODE defined, as described below, and I'm not 100% sure on 64 bit support).

# Bugs / string handling

If compiled with UNICODE defined (e.g. by a compiler flag), a lot of stuff will break:

• The get_process_id_... family of functions will fail to return the correct process id (the internally called functions expect wide strings, you provide ASCII ones).
• inject_dll will try to call GetModuleHandle with a hardcoded ASCII string, which it won't expect. This can be handled with the TEXT macro.
• Also in inject_dll, strlen might not return the expected value if given a wide character string.
• Once that's fixed, the remotely created thread will not execute as intended, as it will pass a wide character string to a LoadLibraryA that is not fit to handle those.

# NULL

NULL is used in several places, both as a 0 integral value and a nullptr pointer value. Try to use more explicit values (especially the more type-safe nullptr for pointer values), as currently every time NULL is involved, I have to double check whether the value represented is a pointer or not.