0

I am trying to compile a program that hooks KiFastSystemCall from NTDLL.dll. I am using inline assembly from a forum I found (I am by no means an assembly professional and I have 0 experience writing in it). The program compiles but crashes while executing the assembly code.

The function:

void set_up_hook() {
    void (*func)() = &new_func;
    BOOL(__stdcall *oldProtection)(LPVOID, SIZE_T, DWORD, PDWORD) = &VirtualProtect;
    std::cout << "started block number 1\n";
    __asm {
        mov esi, 07FFE0300h
        lodsd
        call changeProtection

    changeProtection :
        push eax
            push oldProtection
            push 40h
            push 6
            push eax
            call VirtualProtect
            pop eax
            retn
    }
    std::cout << "finished block 1\n";
    __asm {
        mov edx, 03EBh
        mov[eax], edx
        lea eax, [eax + 5]
        mov dl, 68h
        mov[eax], dl
    }
    std::cout << "finished block 2\n";
    __asm {
        inc eax
        mov edx, func
        mov[eax], edx
    }
    std::cout << "finished block 3\n";
    __asm {
        lea eax, [eax + 4]
        mov dl, 0C3h
        mov[eax], dl
    }
    std::cout << "done!";
}

I have divided the code into blocks to see where it crashed and it crashes at the very first block. It didn't work before as well so I don't think the division into blocks is the problem.

Thanks in advanced :D

[EDIT] I found this code with comments that I deleted because for some reason visual studio gave me errors that I couldn't solve on my own.

Here is the original code with comments:

.386
.model flat,stdcall
option casemap:none

include kernel32.inc
includelib kernel32.lib

.data

oldProtection dd ? 

fileToDelete db "C:\Temp\deleteMe.txt", 0 ; Create this file or change the path and check if it was deleted.

; Array listing all the hooks we install.
; Each hook is placed according to its function's syscall number.
arrayOfEvil DWORD 149h DUP (0), offset newNtSetInformationFile , 40h DUP (0)

.code

start:
    mov esi, 07FFE0300h
    lodsd                       ; EAX = KiFastSystemCall
    call changeProtection       ; Not changing the protection back is bad for your health
    mov edx, 03EBh              ; 0xEB06 JMP SHORT 0xE bytes
    mov [eax], edx
    
    lea eax, [eax + 5h]         ; EAX = [KiFastSystemCallRet + 1]
    mov dl, 68h                 ; 0x68 = PUSH
    mov [eax], dl
    
    inc eax
    
    mov edx, offset evilCode    ; EDX = Pointer to our trap
    mov [eax], edx              ; [KiFastSystemCallRet] = PUSH offset evilCode
    
    lea eax, [eax + 4]  
    mov dl, 0C3h                ; 0xC3 = RETN
    mov [eax], dl
    
    push offset fileToDelete
    call DeleteFile             ; Will call NtSetInformationFile
    
    retn
    
    
    changeProtection:
        push eax                    ; Save KiFastSYstemCall addr
        push offset oldProtection
        push 40h                    ; PAGE_EXECUTE_READWRITE
        push 0Ah                    
        push eax
        call VirtualProtect         ; VirutalProtect((void *)KiFastSystemCall, 10, PAGE_EXECUTE_READWRITE, &oldProtection
        pop eax
        retn
    
    evilCode:
        mov ecx, offset arrayOfEvil
        lea ecx, [ecx + eax * 4]
        mov ebx, [ecx]
        cmp ebx, 0
        jz origKiFastSystemCall
        jmp ebx
        
    newNtSetInformationFile:
        pushad
        mov edi, [esp + 38h]
        cmp edi, 0Dh                ; 0xD = FileDispositionInformation
        jnz callRealKiFastSystemCall
        xor edi, edi
        mov ebx, [esp + 30h]        ; EBX = (VOID *)dispositionInfo
        mov [ebx], dl               ; dispositionInfo.DeleteFile = 0 (FALSE)
    callRealKiFastSystemCall:
        popad   
        jmp origKiFastSystemCall
        
    origKiFastSystemCall:
        mov edx, esp
        dw 340fh                    ; SYSENTER
        retn

end start

This code is pure assembly and I wanted to integrate it into my c++ code. Also Instead of calling the evilcode routine I tried to change it so it would call my new_func. As for the other functions I use, VirtualProtect() is a winapi function and new_func() is just a demo function I created to see if the hook works, all it does is ''' std::cout << "hook worked\n"; '''

Hope this is enough info to solve the problem

  • Please try to create a [mre] to show us. You use and call a lot of functions that we can't see. Are you sure the crashes are not in those functions? Have you tried to use a [debugger](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) to find the true location of the crash? – Some programmer dude Oct 23 '22 at 17:24
  • This code has far too few comments for assembly code. It's pure guess what it's supposed to do, let alone what it does. Things like `mov esi, 07FFE0300h` are generally fishy. Where did you get that address from? – PMF Oct 23 '22 at 17:24
  • 2
    And why do you use inline assembly to begin with? What is the problem that's supposed to solve? – Some programmer dude Oct 23 '22 at 17:24
  • Crashes on what instruction, with what register values? Why do you `call changeProtection` when that's the next instruction, and then have execution return and fall into `changeProtection:` again? So you end up running a `ret` from the middle of an `asm{}` block, with the stack pointer pointing wherever the compiler had it. – Peter Cordes Oct 23 '22 at 17:28
  • 1
    Also you seem to expect EAX to keep its value between `asm{}` blocks, which isn't guaranteed. And definitely won't happen in practice when there's a function call like `std::cout << "stuff\n"` between blocks. – Peter Cordes Oct 23 '22 at 17:30
  • [Visual Studio (MASM) assembly - why does code in labels automatically execute, even if label not called](https://stackoverflow.com/q/70268937) and [How do labels execute in Assembly?](https://stackoverflow.com/q/66012557) explain the fall-through problem into `changeProtection` after `call changeProtection` returns. Just don't call / ret in the first place. As for the larger question of whether there are other bugs, well probably. Use a debugger to single-step and see how your code executes; that would have made this bug clear. As for why you need inline asm for this IDK, unclear. – Peter Cordes Oct 23 '22 at 17:35
  • 1
    Why is the pointer of `VirtualProtect` is passed into `VirtualProtect` as the `lpflOldProtect` argument while it expects a pointer to a dword that will hold the value of the old protection? There are so many things wrong with your code, you should not use inline asm unless you absolutely know what you're doing. – thedemons Oct 23 '22 at 17:44
  • I didn't find a duplicate for the other major bug, of assuming the compiler won't use registers between your asm blocks. Reluctant to reopen due to lack of comments about how this is supposed to work overall, though, like what's up with the various stores, and what VirtualProtect call is actually being made, and what the point of inline asm is vs. just `volatile char*`. – Peter Cordes Oct 23 '22 at 17:45
  • @Someprogrammerdude I updated the code and added the original. I am using inline assembly because trying this with regular c++ didn't work (I do have experience doing userland hooking with c++) and I saw this example so I thought I'll give it a shot – Omer Cohen Oct 23 '22 at 18:25
  • @PMF updated post with original code, comments and more explanations. – Omer Cohen Oct 23 '22 at 18:26
  • @PeterCordes as I said I have 0 experience with assembly, I'll remove the extra call to ChangeProtection, I thought the second one was just a definition. Regarding the retn, it was like that in the original code and I don't know what will removing it do so I didn't, if you have a fix I'll be glad to hear. – Omer Cohen Oct 23 '22 at 18:29
  • maybe you can explain what problem you're trying to solve? Trying to solve a problem with a piece of code you don't understand is very rarely the way to go. – PMF Oct 23 '22 at 19:44

0 Answers0