C0D3-M4513R / inject-lib

https://docs.rs/inject-lib
GNU Lesser General Public License v2.1
6 stars 0 forks source link

eject failed #29

Closed whklhh closed 2 years ago

whklhh commented 2 years ago

Thank u for this project. My English is bad, sorry. There are two errors in InjectWin::eject:

  1. windows::str_from_wide_str convert raw wide char pointer to rust-string without trim '\x00', which will make get_module_in_pid cannot find target module handle. I add .map(|s|{String::from(s.trim_end_matches(char::from(0)))}) to fix this.
  2. The arg lpParameter of CreateRemoteThread will become the arg of lpStartAddress directly. When lpStartAddress is 'FreeLoadLibraryA', lpParameter should be the handle instead of a pointer.
C0D3-M4513R commented 2 years ago

Thank you for the report! Glad to know, that someone is using my lib. I will look into the errors later today. I will re-read the documentation of CreateRemoteThread again.

C0D3-M4513R commented 2 years ago

I assume, that the dll name, that the user provides is not null terminated. As for the module names read by Windows functions, I actually explicitly trim null terminators from the chars here.

Given those two things, I do not see how the comparison function can fail.

C0D3-M4513R commented 2 years ago

As for the issue about CreateRemoteThread, I really do not see an issue there. For lpParameter it is stated in the Documentation, that it is:

A pointer to a variable to be passed to the thread function.

Also FreeLoadLibraryA does not exist. FreeLibrary,FreeLibraryAndExitThread and LoadLibraryA exist. And I put the handle here in the Memory page, that will get referenced in CreateRemoteThread .So far I do not see anything wrong with the things you pointed out.

whklhh commented 2 years ago

InjectWin::eject use str_from_wide_str without trim_wide_str. Maybe trim_wide_str should be put in the str_from_wide_str to avoid forgetting.

Sorry for spelling FreeLibrary wrong. The question is, lpStartAddress should point to

the application-defined function of type LPTHREAD_START_ROUTINE

which will accepting the parameter pointer. While exec_fn_in_proc omit this step. So the param will become the arg of target function directly. This is hacky but working, only when the type of target function is also LPTHREAD_START_ROUTINE. That is also the reason of LoadLibraryA caller didn't put the pointer of the dll-name-str into the arg but still work correctly.

C0D3-M4513R commented 2 years ago

As for eject, you are right. I will replace get_module_in_pid with a call to get_module. This has been an oversight over many iterations that have gone into developing this project (and eject being less of a priority than inject).

Indeed I am not validating, that the entry_fn in exec_fn_in_proc is of type LPTHREAD_START_ROUTINE. That is intended, because I know of no possible way to verify that (and the function is private anyways). Putting the pointer of dll_name or path directly as lpParameter wouldn't work, because the pointers are in the scope of the injector programm, but CreateRemoteThread expects the pointer to be in the scope of the target process afaik.

So to solve this I create a bit of memory in the target process using MemPage. After that I fill the memory with the argument, that I wish to pass to the entry_fn. MemPage will then take care of pointers, and free'ing the memory.

whklhh commented 2 years ago

I mean, the correct way to call CreateRemoteThread is:

// in the target process, so it must be injected as shellcode
DWORD WINAPI ThreadProc(
  _In_ LPVOID lpParameter
)
{
    // lpParameter = pointer_buf, so it can get more args
    char* dll_name = (char**)lpParameter[0];
    // other_args = lpParameter[...];
    LoadLibraryA(dll_name);
    return 0;
}

// in this process
void main()
{
  char dll_name[] = "example.dll";
  void* dll_name_buf = VirtualAllocEx(..., sizeof dll_name, ...);
  void* pointer_buf = VirtualAllocEx(..., sizeof dll_name_buf, ...);
  WriteProcessMemory(..., dll_name_buf, dll_name, ...);
  WriteProcessMemory(..., pointer_buf, dll_name_buf, ...);
   // if there are more than 1 args, other args need to be written in pointer_buf[2...] 
  CreateRemoteThread(..., ThreadProc, pointer_buf, ...);
}

But mostly we omit the ThreadProc, which parse the lpParameter. So in this situation(lpStartAddress = LoadLibraryA, FreeLibrary, target_library_func, etc...), the number of args passed cann't be more than 1.

In other words, the arg of Remote-LoadLibraryA is &dll_name, which is same as the arg of Local-LoadLibraryA. While the arg of Remote FreeLibrary is &handle, which is different with Local-FreeLibrary.

C0D3-M4513R commented 2 years ago

Content wise I am passing a char* for inject, and a HMODULE* for eject. The appearing type is void*. I tested making eject pass HMODULE** to CreateRemoteThread, but that did not change anything (CreateRemoteThread still executes as expected, and the dll gets attached and detached to a thread).

What is your actual issue? Is it, that the dll is not actually unloaded? That I can only blame on windows. Previously it worked. I just took a look at how ProcessHacker unloads, and they use ntdll. I could try and make eject use ntdll as well, but that requires restructuring of the internals (something, that I could do). Also of course that feature would be behind the ntdll flag.

whklhh commented 2 years ago

Yes, the dll is not unloaded. And I fixed it by modifying the address of Mempage to the handle. Then it's working. So I think the lpParameter shoud be the handle itself. I think it's wrong to pass char for char(LoadLibraryA) and HMODULE* for HMODULE(FreeLibrary). The arg of FreeLibrary should be the handle instead of the pointer.

C0D3-M4513R commented 2 years ago

I implemented what you suggested in #30. For me this fixed library unloading, and it should now work again. Thanks for the issue! I will wait for a release until I can figure #25 out. I fucked something there.

C0D3-M4513R commented 2 years ago

Released v.0.3.1