win32 c++ acces violation when executing shellcode

Question

I wrote this function to find out the length of an assembly instruction based on this talk about breaking the x86 instruction set.

/* shellcode is a pointer to a buffer contains assembly instructions
   size is the size of that buffer */
DWORD GetInstructionLength(BYTE* shellcode,SIZE_T size) {
    LPVOID RWXBuff = nullptr; //buffer with read,write,execute perm
    LPVOID RWBuff = nullptr;  //buffer with read,write perm
    HANDLE CurProc = GetCurrentProcess();
    DWORD Tmp;
    DWORD CurrOffset = 1;
    RWXBuff = VirtualAllocEx(CurProc, NULL, 32, MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE);
    RWBuff = RWXBuff; 
    for (int i = 0; i < 16; i++, RWBuff = static_cast<char*>(RWBuff) + 1);
    VirtualProtectEx(CurProc, RWBuff, 17, PAGE_READWRITE, &Tmp);
    void (*func)();

    for (;;CurrOffset++) {
        __try {
            LPVOID ShellcodeAddr = static_cast<char*>(RWXBuff) + 16 - CurrOffset;
            std::cout << "memcpy(" << ShellcodeAddr << ", " << (void*)shellcode << ", " << size << ")\n";
            memcpy(ShellcodeAddr, shellcode, size);
            std::cout << "memcpied" << std::endl;
            MEMORY_BASIC_INFORMATION minfo;
            VirtualQueryEx(GetCurrentProcess(), ShellcodeAddr, &minfo, sizeof(MEMORY_BASIC_INFORMATION));
            if (PAGE_EXECUTE_READWRITE == minfo.Protect)
                std::cout << "shellcode at " << ShellcodeAddr << "is executable" << std::endl;
            else
                std::cout << "shellcode at " << ShellcodeAddr << "is NOT executable" << std::endl;
            func = (void(*)())ShellcodeAddr;
            func();
            break;
        }
        __except (GetExceptionCode() == EXCEPTION_ACCESS_VIOLATION ? EXCEPTION_EXECUTE_HANDLER : EXCEPTION_CONTINUE_SEARCH) {
            std::cout << "Bug!" << std::endl;
            continue;
        }
    }
    VirtualFreeEx(CurProc, RWBuff, 0, MEM_RELEASE);
    return CurrOffset;
}

Caller:

int main()
{
    unsigned char shellcode = 0x90; // nop opcode
    unsigned char* buf = &shellcode;
    size_t size = helper::GetInstructionLength(buf, 1);
    std::cout << "Shellcode lenght = " << size << std::endl;
}

Output:

buffer with rwx at 0000023048E80000
buffer with rw- at 0000023048E80010
shellcode at 0000023048E8000F is NOT executable
Bug!
shellcode at 0000023048E8000E is NOT executable
Bug!
shellcode at 0000023048E8000D is NOT executable
Bug!
shellcode at 0000023048E8000C is NOT executable
Bug!
shellcode at 0000023048E8000B is NOT executable
Bug!
shellcode at 0000023048E8000A is NOT executable
Bug!
shellcode at 0000023048E80009 is NOT executable
Bug!
shellcode at 0000023048E80008 is NOT executable
Bug!
shellcode at 0000023048E80007 is NOT executable
Bug!
shellcode at 0000023048E80006 is NOT executable
Bug!
shellcode at 0000023048E80005 is NOT executable
Bug!
shellcode at 0000023048E80004 is NOT executable
Bug!
shellcode at 0000023048E80003 is NOT executable
Bug!
shellcode at 0000023048E80002 is NOT executable
Bug!
shellcode at 0000023048E80001 is NOT executable
Bug!
shellcode at 0000023048E80000 is NOT executable
Bug!
Bug!
...

But it won't work and will raise acces violation every time and I have no idea why. Can you help me out with this? Because I only have 3 months of coding experience, sorry if this look like bad code to you

Lots of odd decisions in this code, like, using the `Ex`-versions with `GetCurrentProcess`. Or a missing [`FlushInstructionCache`](https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-flushinstructioncache). Or the abundant lack of error checking. Also, you are leaving us guessing, what code you are trying to execute. `shellcode` is something. That something fails. Would it not be reasonable to allow us to actually see that something? — IInspectable, May 20 '21 at 09:09
@IInspectable: x86 has [coherent instruction cache](https://stackoverflow.com/questions/17395557); the only reason GCC needs you to use `__builtin____clear_cache()` is so it knows the stores aren't "dead", and have to actually be in memory before calling through a function pointer. ([How does \_\_builtin\_\_\_clear\_cache work?](https://stackoverflow.com/posts/comments/85964322)). IDK if MSVC is similar with ` FlushInstructionCache` being needed (but zero asm instructions), or if it would just work without that or `volatile`. — Peter Cordes, May 20 '21 at 16:02

Peter Cordes · Accepted Answer · 2021-05-22T17:25:33.560

rwx at ...80000 /
rw- at ...80010 - those are in the same page.
x86 memory protection works at page granularity (4096B), so you just changed the whole page to non-executable, including the RWXBuff.

The end of the ...80000 page is 80FFF, and the start of the next is ...81000.
(12 offset-within-page address bits = 3 hex digits.)

Note that MSVC has a FlushInstructionCache function, like GNU C's __builtin____clear_cache(). In GNU C, that's not optional even on x86, despite x86 having coherent instruction cache. The reason you need it for safety after storing machine-code bytes into a buffer is so the compiler knows those stores can't be optimized away (or reordered) if it doesn't see any data accesses to them.

MSVC is the same: you need to call FlushInstructionCache after C assignment of machine code bytes to an array, before dereffing a function-pointer to that array.

It will often work without on x86 (including x86-64), unlike on ARM and other ISAs without coherent I-cache, but it's possible for it not to, especially if you assign different code byte to the buffer and call it again. (That's likely to tempt the compiler into eliminating the dead store and only doing the later store. At least if it's sure the memory is private to this function, not globally reachable from whatever unknown function it's calling.)

[If FlushInstructionCache doesn’t do anything, why do you have to call it, revisited](https://devblogs.microsoft.com/oldnewthing/20190902-00/?p=102828). — IInspectable, May 22 '21 at 16:36

win32 c++ acces violation when executing shellcode

1 Answers1