0

I try to hook into NtWriteFile. Below you find a stripped version of the code I wrote for a dll. The idea is to load the resulting dll with the withdll.exe of MS Detours. With some debugging I found that MyNtWriteFile gets indeed called but then gets stuck at the point of the original function call (the RealNtWriteFile call). Any hints on why is that are highly appreciated. :)

#include "pch.h"

#include<windows.h>
#include <detours.h>
#include <stdio.h>
#include <iostream>
#include <winternl.h>


typedef NTSTATUS(*NtWriteFileFunc)(
    HANDLE FileHandle,
    HANDLE Event,
    PIO_APC_ROUTINE ApcRoutine,
    PVOID ApcContext,
    PIO_STATUS_BLOCK IoStatusBlock,
    PVOID Buffer,
    ULONG Length,
    PLARGE_INTEGER ByteOffset,
    PULONG Key
    );


NTSTATUS WINAPI MyNtWriteFile(
    HANDLE           FileHandle,
    HANDLE           Event,
    PIO_APC_ROUTINE  ApcRoutine,
    PVOID            ApcContext,
    PIO_STATUS_BLOCK IoStatusBlock,
    PVOID            Buffer,
    ULONG            Length,
    PLARGE_INTEGER   ByteOffset,
    PULONG           Key
)
{
    // Call the original function.
    NtWriteFileFunc RealNtWriteFile = (NtWriteFileFunc)GetProcAddress(LoadLibrary(L"ntdll.dll"), "NtWriteFile");
    NTSTATUS tmp = RealNtWriteFile(FileHandle, Event, ApcRoutine, ApcContext,
        IoStatusBlock, Buffer, Length, ByteOffset, Key);
    
    return tmp;
}

BOOL WINAPI DllMain(HINSTANCE hinst, DWORD dwReason, LPVOID reserved)
{   
    HMODULE hNtdll = LoadLibrary(L"ntdll.dll");
    
    NtWriteFileFunc RealNtWriteFile = (NtWriteFileFunc)GetProcAddress(hNtdll, "NtWriteFile");
    
    LONG error;

    if (DetourIsHelperProcess()) {
        return TRUE;
    }

    if (dwReason == DLL_PROCESS_ATTACH) {
        DetourRestoreAfterWith();
        DetourTransactionBegin();
        DetourUpdateThread(GetCurrentThread());
        DetourAttach(&(PVOID&)RealNtWriteFile, MyNtWriteFile);
        error = DetourTransactionCommit();

        
    }
    else if (dwReason == DLL_PROCESS_DETACH) {
        DetourTransactionBegin();
        DetourUpdateThread(GetCurrentThread());
        DetourDetach(&(PVOID&)RealNtWriteFile, MyNtWriteFile);
        error = DetourTransactionCommit();
        
    }
    return TRUE;
}
johannes
  • 11
  • 2
  • [Dynamic-Link Library Best Practices](https://learn.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-best-practices). – IInspectable Jan 09 '23 at 00:33
  • Deadlock. Don't call `LoadLibrary` inside `MyNtWriteFile` (or any other Win32 function). `LoadLibrary` does so much that it wouldn't surprise me if it tries to write something. – Axalo Jan 09 '23 at 00:54
  • Thanks for the advice. I added a ```#pragma comment(lib, "ntdll.lib")``` at the beginning now and deleted the ```LoadLibrary``` call. From how I understood things, ```GetModuleHandle``` is okay to use in this context. This is how I got the handle now. Besides that nothing has changed and the problem also remains :/ – johannes Jan 09 '23 at 01:47
  • @Axalo - here no any deadlock. and can not be in pricip ( even if LoadLibrary call NtWriteFile - which it not doing here) – RbMm Jan 09 '23 at 02:17
  • 1
    @RbMm I wasn't talking about `DllMain`. We have to assume the things that aren't documented. We have to assume that `LoadLibrary` calls `NtWriteFile`. We also have to assume that the mutexes it locks aren't reentrant. If you now call `LoadLibrary` and it calls `NtWriteFile`, due to your hook, you're calling `LoadLibrary` inside `LoadLibrary`. And because the mutexes aren't reentrant, you have a deadlock. That was my thought process. It doesn't matter that you know exactly how `LoadLibrary` is implemented if there are no guarantees made by Microsoft. The implementation can change at any time. – Axalo Jan 09 '23 at 13:42
  • @Axalo call `LoadLibrary` inside `LoadLibrary` is full ok. nothing not reetrant exist here. and if `LoadLibrary` call `NtWriteFile` ( it really exactly not do it) - wil be infinite recursion (if not check for it) but not hang (deadlock) – RbMm Jan 09 '23 at 16:43

1 Answers1

2

call RealNtWriteFile is fundamental error. because this lead to infinite reqursive loop. you need use pointer , modified in call DetourAttach, for call original function.

at first use static link with ntdll.lib - not need GetProcAddress.

than declare ( in x64, in x86 need small additional trick) next variable:

EXTERN_C extern PVOID __imp_NtWriteFile;

you need change protect of this variable:

VirtualProtect(&__imp_NtWriteFile, sizeof(PVOID), PAGE_EXECUTE_READWRITE, &op);

if you detour several function - better first get self IAT section and change protect of all IAT, for not do this several times ( RtlImageDirectoryEntryToData(&__ImageBase, TRUE, IMAGE_DIRECTORY_ENTRY_IAT, &size); )

and use next call

DetourDetach(&__imp_NtWriteFile, MyNtWriteFile);

restore protection of _imp / IAT ( optional )

and inside MyNtWriteFile, if you want call original function - simply call NtWriteFile as is.

sense of all this is next - __imp_NtWriteFile initially will be hold address of ntdll!NtWriteFile ( this do loader )

the DetourAttach(&__imp_NtWriteFile, myhook) - set hook in address to which point __imp_NtWriteFile and modify this pointer (it Inout ) parameter. after (success) call __imp_NtWriteFile will be point to tramopline ( chunk of memory - where several original bytes or hooked function saved + jmp to function body after this bytes)

and NtWriteFile use value stored at variable __imp_NtWriteFile for call api. main that api must be declared with __declspec(dllimport)

this is common - for imported someapi used PVOID __imp_someapi variable. if you use delayed import - __imp_load_someapi name is used (but not in x86)

if by some reason (really not need do this) want not static link to ntdll - anyway declare and define in this case

EXTERN_C PVOID __imp_NtWriteFile = 0;

note, that already variable not extern (declared only) but defined.

and you need now direct call __imp_NtWriteFile = GetProcAddress(GetModuleHandleW(L"ntdll.dll"), "NtWriteFile");

and now you not need VirtualProtect of course.

for x86 - name is mangled it will be

__imp__NtWriteFile@36

unfortunatelly we can not direct use names with @ symbol in c/c++ code. so possible 2 solution - use asm - in it we can have such names and call DetourAttach from asm.

but more simply solution, use /ALTERNATENAME linker option.

so use

#ifdef _X86_
#pragma comment(linker, "/ALTERNATENAME:___imp_NtWriteFile=__imp__NtWriteFile@36")
#endif

in case you static link to ntdll - the variable __imp__NtWriteFile@36 is exist - it defined by linker. but we can not access it in cpp. instead we use ___imp_NtWriteFile defined as extern. it not exist and we tell linker use __imp__NtWriteFile@36

if you not static link to ntdll, but defined __imp_NtWriteFile by self - need inverted declaration

#ifdef _X86_
#pragma comment(linker, "/ALTERNATENAME:__imp__NtWriteFile@36=___imp_NtWriteFile")
#endif

because in this case already __imp__NtWriteFile@36 not exist and need use ___imp_NtWriteFile in it place


and what you can do inside hook: of course no sense set hook for only and call original api. so real code will be do something more. and here exist risk or reqursive call - in hook you call some api , and this api indirect again call your hook. for this you need detect reqursive call and in such case - direct call original api, without any extra processing. for this can be used RtlGetFrame, RtlPushFrame, RtlPopFrame or tls. but this is already separare question

RbMm
  • 31,280
  • 3
  • 35
  • 56
  • Thank you for your help. I see the problem now but one question remains: you said I can just use ```NtWriteFile``` inside ```MyNtWriteFile```. But since ```NtWriteFile``` is using the value stored at ```__imp__NtWriteFile```, isn't that a loop again? – johannes Jan 09 '23 at 08:46
  • @johannes - are you read my answer ? `DetourAttach` is **modify** value of `__imp__NtWriteFile` – RbMm Jan 09 '23 at 09:03
  • Yes, I did read it. The way I understood it, ```__imp__NtWriteFile``` points to ```MyNtWriteFile``` after the modification. – johannes Jan 09 '23 at 09:17
  • @johannes no of course. after `DetourAttach` return - it point to trampoline, which you need call – RbMm Jan 09 '23 at 09:20
  • @johannes also call to `DetourUpdateThread(GetCurrentThread());` is senseless, butnothing affect – RbMm Jan 09 '23 at 09:21
  • so what I did now in summary: 1. added ```#pragma comment(lib, "ntdll.lib")``` 2. ```EXTERN_C extern PVOID __imp_NtWriteFile;``` 3. ```DWORD op; VirtualProtect(&__imp_NtWriteFile, sizeof(PVOID), PAGE_EXECUTE_READWRITE, &op);``` 4. replace ```RealNtWriteFile``` with ```NtWriteFile``` but here it gives me an error saying that it is undefined. I would like to chat about this but my reputation is only 11 :/ – johannes Jan 09 '23 at 09:43
  • @johannes and are you define `NtWriteFile` ? you not include *ntifs.h*. so you must define it by self – RbMm Jan 09 '23 at 09:49
  • yes, I now defined it. But I get the error _unresolved external symbol_. – johannes Jan 09 '23 at 10:18
  • @johannes - I don't know how you defined it and which symbol is unresolved. You forget that i not under your monitor. And in all case task required understanding – RbMm Jan 09 '23 at 10:26
  • I defined it like this: ` ``__kernel_entry NTSYSCALLAPI NTSTATUS NTAPI NtWriteFile( _In_ HANDLE FileHandle, _In_opt_ HANDLE Event, _In_opt_ PIO_APC_ROUTINE ApcRoutine, _In_opt_ PVOID ApcContext, _Out_ PIO_STATUS_BLOCK IoStatusBlock, _In_reads_bytes_(Length) PVOID Buffer, _In_ ULONG Length, _In_opt_ PLARGE_INTEGER ByteOffset, _In_opt_ PULONG Key );``` The error is more specific:LNK2019 unresolved external symbol "__declspec(dllimport) long __cdecl NtWriteFile(void *,.... – johannes Jan 09 '23 at 10:42
  • What exactly name of the symbol ? Extern "C" you of course forget? – RbMm Jan 09 '23 at 11:02
  • Do i need to add ```EXTERN_C``` when defining ```NtWriteFile```? This is the exact error: Severity Code Description Project File Line Suppression State Error LNK2019 unresolved external symbol "__declspec(dllimport) long __cdecl NtWriteFile(void *,void *,void (__cdecl*)(void *,struct _IO_STATUS_BLOCK *,unsigned long),void *,struct _IO_STATUS_BLOCK *,void *,unsigned long,union _LARGE_INTEGER *,unsigned long *)" (__imp_?NtWriteFile@@YAJPEAX0P6AX0PEAU_IO_STATUS_BLOCK@@K@Z010KPEAT_LARGE_INTEGER@@PEAK@Z) referenced in function... – johannes Jan 09 '23 at 11:07
  • @johannes of course need. Are you not view ? – RbMm Jan 09 '23 at 11:14