Module scanner assumes memory region must be executable

The ScanForModules_MemoryWalk_Hidden implementation makes an incorrect assumption around the memory layout of modules in memory.

The relevant code is as follows:

if (GetModuleHandleEx(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, (TCHAR*)addr, &moduleHandle) == FALSE)
{
    // not a known module. check if this is executable.
    if ((region->Protect & PAGE_EXECUTE) == PAGE_EXECUTE ||
        (region->Protect & PAGE_EXECUTE_READ) == PAGE_EXECUTE_READ ||
        (region->Protect & PAGE_EXECUTE_WRITECOPY) == PAGE_EXECUTE_WRITECOPY ||
        (region->Protect & PAGE_EXECUTE_READWRITE) == PAGE_EXECUTE_READWRITE)
    {
        auto moduleData = static_cast<PBYTE>(region->AllocationBase);
        if (moduleData[0] == 'M' && moduleData[1] == 'Z')
        {
            // ...

This assumes that the memory region that contains the PE header is executable, but this would only be the case if the injector incorrectly set the entire memory region to be executable. In practice the PE header is usually located in a PAGE_READONLY region.

The check should be modified to look for MZ at the start of any committed, readable region.

I'll patch this after we merge my giant commit for changing the return type for all checks.

ayoubfaouzi / al-khaser

Module scanner assumes memory region must be executable #237