Thread: DLL Injection

Hi people!

First of all, I'd like to say thank you for a wonderful site and forum! It has helped me a lot in my progress in C. Big kudos to you all!

Now on to my question, I'm currently writing an application that injects a DLL in a process of choice, but having some minor problems...

All my code builds fine, and runs, AND it injects the DLL. But as soon as either the DLL has finnished executing or the application (Don't know which) the process I injected into chrashes... And I can't seem to figure out why.

Anyways, here's my code:

run.c

Code:

#include <windows.h>
#include <stdio.h>
#include <psapi.h>
#include <tchar.h>

#define WIN32_LEAN_MEAN

int Inject(DWORD, LPCSTR);

DWORD GetProcessByFileName(char* name)
{
    DWORD process_id_array[1024];
    DWORD bytes_returned;
    DWORD num_processes;
    HANDLE hProcess;
    char image_name[256];
    char buffer[256];
	int i;
    DWORD exitcode;
    EnumProcesses(process_id_array, 256*sizeof(DWORD), &bytes_returned);
    num_processes = (bytes_returned/sizeof(DWORD));
    for (i = 0; i < num_processes; i++) {
        hProcess=OpenProcess(PROCESS_ALL_ACCESS,TRUE,process_id_array[i]);
        if(GetModuleBaseName(hProcess,0,image_name,256)){
            if(!stricmp(image_name,name))
            {
                CloseHandle(hProcess);
                return process_id_array[i];
            }
        }
        CloseHandle(hProcess);
    }
    return 0;
}

int main()
{
	DWORD dwPID;
		dwPID = GetProcessByFileName("explorer.exe");
	if(dwPID == 0)
	{
		STARTUPINFO si = {sizeof(STARTUPINFO)};
		PROCESS_INFORMATION pi;
		CreateProcess(NULL, "C:\\WINDOWS\\explorer.exe", NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi);
		Sleep(1500);
		dwPID = GetProcessByFileName("explorer.exe");
	}

	if(dwPID != 0)
		Inject(dwPID, "C:\\testdll.dll");
	return;
}

int Inject(DWORD ProcID, LPCSTR szDllPath)
{
	LPVOID	lpRemoteMemory;
	HANDLE	hRemoteThread;
	HWND	hOpenProcess;
	SIZE_T	nSize = strlen(szDllPath);
	unsigned long IDProcess = ProcID;
	HMODULE hKernel;

// OpenProcess() - retrieve a HANDLE to the remote process

	hOpenProcess = OpenProcess(PROCESS_ALL_ACCESS, FALSE, IDProcess);
	if(hOpenProcess == NULL)
	{
		printf("OpenProcess is NULL");
		printf("Error: &#37;d\n", GetLastError());
	}
	printf("OpenProcess: %d\n", hOpenProcess);

// VirtualAllocEx() - Allocate memory in remote process addres-space

	lpRemoteMemory = VirtualAllocEx(hOpenProcess, 0, strlen(szDllPath), MEM_RESERVE|MEM_COMMIT, PAGE_EXECUTE_READWRITE);
	if(!lpRemoteMemory)
	{
		printf("VirtualAlloc() Error: %d\n", GetLastError());
		return -1;
	}
	printf("VirtualAlloc(): 0x%x\n", lpRemoteMemory);

// WriteProcessMemory() - Copy initialised injection data strucuture to allocated memory

	if(!WriteProcessMemory(hOpenProcess, lpRemoteMemory, (LPVOID)szDllPath, strlen(szDllPath), NULL))
	{
		printf("WriteProcessMemory() error: %d\n", GetLastError());
	}
	printf("WriteProcessMemory()  Succeeded :)\n\n");
	printf("Size of DLL: %d\n", nSize);

// GetModuleHandle() - kernel32.dll API CALL

	hKernel = GetModuleHandle("KERNEL32.DLL");
	if(!hKernel)
	{
		printf("KernelModule Error: %d\n", GetLastError());
	}
	printf("KernelModule loaded KERNEL32.DLL!\n");

// CreateRemoteThread() - Start the remote copy

	hRemoteThread = CreateRemoteThread(hOpenProcess, NULL, 0, (LPTHREAD_START_ROUTINE)LoadLibrary(szDllPath), lpRemoteMemory, 0, &IDProcess);
	hRemoteThread = CreateRemoteThread(hOpenProcess, NULL, 0, (LPTHREAD_START_ROUTINE)GetProcAddress(hKernel, "LoadLibraryA"), lpRemoteMemory, 0, NULL);
	if(!hRemoteThread)
	{
		printf("CreateRemoteThread Error: %d\n", GetLastError());
		return -1;
	}
	printf("CreateRemoteThread(): %d\n", hRemoteThread);

// No luxury poop, have to clean up
// 1. Close Handle
	if(!CloseHandle(hRemoteThread) && !CloseHandle(hOpenProcess))
	{
		printf("Handle NOT closed!\n");
		printf("Error Closeing: %d\n", GetLastError());
	}
	printf("CloseHandle() finished\n");

// 2. Wait for the thread to complete
	if(!WaitForSingleObject(hRemoteThread, INFINITE))
	{
		printf("WaitThreadObject Failed!\n");
		printf("Error Code: %d\n", GetLastError());
	}
	printf("WaitThreadObject Complete!\n");

// 3. Free memory in remote process
	if(!VirtualFreeEx(hOpenProcess, lpRemoteMemory, 0, MEM_RELEASE))
	{
		printf("VirtualFreeMemory Failed!");
		printf("Error Code: %d\n", GetLastError());
	}
	printf("VirtualFreeMemory Complete!\n");

	return 0;
}

And here's my DLL:

Code:

#include <windows.h>
#include <winsock2.h>
#include <stdio.h>

BOOL APIENTRY DllMain (HINSTANCE hInst     /* Library instance handle. */ ,
                       DWORD reason        /* Reason this function is being called. */ ,
                       LPVOID reserved     /* Not used. */ )
{
printf("Lol\n");
}

run.c is compiled like this: i586-mingw32msvc-gcc run.c -o run.exe -lpsapi -s
under linux, since I'm crosscompiling and used to the linux system...

I compile my DLL with Dev-C++ under windows

Do you guys see any error in my code? Have I forgotten something? Also, please come with constructive critics - as this is my first "major" piece of code...

Free hugs to those who answers!
n1mda

EDIT: This is the ouput when I run "run.exe"

Code:

L:\home\n1mda\programming\testdll>run.exe
OpenProcess: 2036
VirtualAlloc(): 0x16f0000
WriteProcessMemory()  Succeeded :)

Size of DLL: 14
KernelModule loaded KERNEL32.DLL!
Lol
CreateRemoteThread(): 2012
CloseHandle() finished
WaitThreadObject Complete!
VirtualFreeMemory Complete!
Lol

You are using handles of thread and process after they are closed. Not very healthy

How do you propose waiting your a handle you closed?

Originally Posted by n1mda

Thank you for your answer!

But is this caused by the DLL or the injection-code itself?

I though since I have WaitForSingleObject() this would not be an error?

WaitForSingleObject should check the state of the kernel object.
If you close the handle - the object will be destructed as soon as thread/process exits

so your WaitForSingleObject will check the state of the destructed kernel object. It is undefined behavior

Read the return value of WaitForSingleObject - It is NOT BOOL

REturn value is DWORD - not int

printing it with %s - just asking for trouble

Code:

DWORD  ret = WaitForSingleObject(hHandle,  INFINITE);

switch(ret)
{
   case WAIT_ABANDONED: /* should not occure with thread handle */
      break;
   case WAIT_OBJECT_0:
      /* wait successful */
      break;
   case WAIT_TIMEOUT:
      /* should not occure with waiting INFINITE */
      break;
   case WAIT_FAILED:
      /* wait failed */
      ret = GetLastError();
      printf("WaitForSingleObject failed with error 0x%x", ret);
      break;
}

You are not listening.
How do you propose to wait for something that you've never started/assigned?
Take a look at your application again. Read that code. Tell us what it does. In order.
And don't just copy and paste code. If you can't figure out something easy like that, then maybe this process isn't for you. Read the documentations and read what they have to say. Then you should be able to understand how functions act and how you should code.
You just copied to wait from what iMalc replied.

And do not use handles after you closed them

Indeed, that was the original problem. But another one to note is:
Don't close handles before the operations on those handles are finished. That is, don't close the handles before the threads finish.
Since CreateRemoteThread will signal the handles when finished, it will signal a closed handle which is never good.

Originally Posted by Elysia

Indeed, that was the original problem. But another one to note is:
Don't close handles before the operations on those handles are finished. That is, don't close the handles before the threads finish.
Since CreateRemoteThread will signal the handles when finished, it will signal a closed handle which is never good.

You wrong
It is safe to close handles before the thread finishing. But after that - you should not use that handle in any system call (You cannot wait for thread exiting, get the thread return code etc)

In the code above VirtualFreeEx was called for the closed handle - it is not good.

Huh, well I'd assumed it was a bad thing.