[solved] C# Dll Injection error Win 7 x64

[adaephon]

I'm attempting to write a DLL Injector in C# using the CreateRemoteThread / LoadLibrary technique. I got to the point last night where the function runs without erroring (until I put in another exception) but doesn't actually inject the module (leading me to believe that LoadLibrary is not actually executed). My machine at home where I was running it is Win 7 Ultimate RC 7100 x64.

I got to work this morning and the code ran, and injected the DLL, without a hitch (work machine is XP SP3 32bit). This reinforces my belief that the problem is something to do with changes between XP and Vista/7 and/or differences in Dll injection between x64/x86.

So far the examples I've seen on injecting in x64 seem pretty similar in terms of what they are doing. I had a look at Cypher's loader program, and the most notable difference in that (other than it being C++) is the CustomGetProcAddress rewrite that parses PE image format to find LoadLibraryW address. As the reasoning behind this is due to Vista potentially stuffing things up, I was wondering if that is likely the problem.

I've attached the relevant code. At home on Win 7 x64 it gets to the "Code executed properly, but unable to get an appropriate module handle, possible Win32Exception" line. Marshal.GetLastWin32Error() returns 0 at this point ("The operation completed successfully" - But this probably just indicates that GetExitCodeThread doesn't cause an error - I'll be testing what possible Win32 errors are returned from the call to CreateRemoteThread once I get home this arvo)

private IntPtr InjectLibraryInternal(string libFullPath)
        {
            uint sizeUni = (uint)Encoding.Unicode.GetByteCount(libFullPath);

            // Get Handle to Kernel32.dll and pointer to LoadLibraryW
            IntPtr hKernel32 = Imports.GetModuleHandle("Kernel32");
            if (hKernel32 == IntPtr.Zero)
                throw new Win32Exception(Marshal.GetLastWin32Error());
            IntPtr hLoadLib = Imports.GetProcAddress(hKernel32, "LoadLibraryW");
            if (hLoadLib == IntPtr.Zero)
                throw new Win32Exception(Marshal.GetLastWin32Error());

            // allocate memory to the local process for libFullPath
            IntPtr pLibPath = Imports.VirtualAllocEx(_process.Handle, IntPtr.Zero, sizeUni, AllocationType.Commit, MemoryProtection.ReadWrite);
            if (pLibPath == IntPtr.Zero)
                throw new Win32Exception(Marshal.GetLastWin32Error());

            // write libFullPath to pLibPath
            int bytesWritten;
            if (!Imports.WriteProcessMemory(_process.Handle, pLibPath, Marshal.StringToHGlobalUni(libFullPath), sizeUni, out bytesWritten) || bytesWritten != (int)sizeUni)
                throw new Win32Exception(Marshal.GetLastWin32Error());

            // load dll via call to LoadLibrary using CreateRemoteThread
            IntPtr hThread = Imports.CreateRemoteThread(_process.Handle, IntPtr.Zero, 0, hLoadLib, pLibPath, 0, IntPtr.Zero);
            if (hThread == IntPtr.Zero)
                throw new Win32Exception(Marshal.GetLastWin32Error());
            if (Imports.WaitForSingleObject(hThread, uint.MaxValue) != 0)
                throw new Win32Exception(Marshal.GetLastWin32Error());

            // get address of loaded module
            IntPtr hLibModule;// = IntPtr.Zero;
            if (!Imports.GetExitCodeThread(hThread, out hLibModule))
                throw new Win32Exception(Marshal.GetLastWin32Error());
            if (hLibModule == IntPtr.Zero)
                throw new Exception("Code executed properly, but unable to get an appropriate module handle, possible Win32Exception", new Win32Exception(Marshal.GetLastWin32Error()));

            // clean up
            if (!Imports.CloseHandle(hThread))
                throw new Win32Exception(Marshal.GetLastWin32Error());

            if (!Imports.VirtualFreeEx(_process.Handle, pLibPath, 0, AllocationType.Release))
                throw new Win32Exception(Marshal.GetLastWin32Error());
            
            return hLibModule;
        }

Edit: Fixed, Platform Target: Any CPU needed to be x86 for the dll that was being injected/process it was being injected into. Thanks Cypher, now to fix resource leaks.

[Cypher]

Your .NET app is probably running as an x64 process. You cannot inject into a 64-bit process from a 32-bit process and vice-versa.

Please check that first and if that doesn't work I'll step you through the other potential problems.

Your code also contains resource leaks. I'll deal with those once the injection bug is fixed.

[adaephon]

Yeh I figured there were probably resource leaks - it's hardly a finished or polished product.

Thanks for the advice, /facepalm for not checking before - Platform Target is Any CPU, which IIRC will cause problems on x64 with interacting with 32bit process. At work at the moment so can't check if modifying that fixes the error, but will try when I get home.

[Cypher]

'Probably'? They're pretty obvious, like a slap in the face.

[adaephon]

Ok, changing target fixed it, thanks again.

As for the pretty obvious resource leaks: I'm closing the handle to the thread, I'm freeing the memory allocated for the Dll path, the process handle is opened/closed outside of this procedure, the module handle from GetModuleHandle (from my understanding) doesn't need to be closed... what have I missed?

Oh wait... Marshal.StringToHGlobalUni... presumably should be storing that pointer and releasing it after the call to WPM...

[Cypher]

You're only freeing the memory and closing the handle if the function succeeds.

But, what if you open the handle, then something goes wrong? An exception is thrown and the handle is leaked.

You're using an OOP language, so start using RAII:
Resource Acquisition Is Initialization - Wikipedia, the free encyclopedia

Or whatever the C# equivalent is. Otherwise you're going to have to litter your code with cleanup stubs before returns and throws.

[adaephon]

Thanks for that, realised the same thing as I was re-looking through it all. I'll have a look into it although I've read that RAII article before (after seeing you mention it somewhere before / reading your EnsureCleanup classes) and I believe it has to be approached somewhat differently in C# due to how GC/resource allocation is managed (pun not intended).

Thanks for the help and advice though

[Cypher]

Oh of course. Everything in C# is on the heap and GCed. Ugh, what a pain in the ass.

[flukes1]

Not really. Just wrap it all in a try/finally construct.

Or create an abstract Handle class that implements IDisposable and wrap your code in a using statement, if you want to be really anal about it.

[adaephon]

Yeh try/finally is what I was thinking, using would be a bit of a pain due to nesting multiple usings for unmanaged resources.

[adaephon]

@Cypher, btw with your CustomGetProcAddress function, what are the vagaries of Vista that you refer to (IAT hooking etc) and how common are they? I.e. my injector seems to work now with the standard winapi GetProcAddress, is it likely to randomly run into errors based on Vista/Win7 internal differences?

[Cypher]

Not really. Just wrap it all in a try/finally construct.

Or create an abstract Handle class that implements IDisposable and wrap your code in a using statement, if you want to be really anal about it.

Of course, I forgot C# had "finally". C++ has totally different idioms so excuse my confusion.

@Cypher, btw with your CustomGetProcAddress function, what are the vagaries of Vista that you refer to (IAT hooking etc) and how common are they? I.e. my injector seems to work now with the standard winapi GetProcAddress, is it likely to randomly run into errors based on Vista/Win7 internal differences?

Vista's ShimEngine (part of the compatibility layer) IAT hooks certain functions. This happens when for example you run an application and set the OS to XP Sp2 or whatever.

The chances of you hitting one of the IAT hooked functions is small, but I always like to be safe. Because if you were to hit one the RVA would be wrong and the remote process would crash. Obviously that's bad.

Fyi the code you are referring to is an early PoC. You'll need to update it to support forwarded functions if you wish to use it in production code. I have an updated version in my own projects but it relies on my PeLib project which is not yet ready to be posted publicly.