Edge - ACG bypass using UnmapViewOfFile - Google, Inc.

Modifying page protections cross-process is an issue, as attacker could possibly unmap JIT code and map in their own read/write memory where we expect JIT code, and trick JIT process into making it executable.
To avoid this we need something to replace our ZeroMemory/VirtualProtectEx(PAGE_NOACCESS) method of mock-decommit (since files have no decommit support), as this later requires a VirtualProtectEx(PAGE_READEXECUTE) to recommit. The solution is to use VirtualUnlockEx, which will serve the same function for us.

Author: MikeHolman

lib/Common/CommonDefines.h

@@ -324,6 +324,7 @@
 
 #ifndef NTBUILD
 #define DELAYLOAD_SECTIONAPI 1
+#define DELAYLOAD_UNLOCKMEMORY 1
 #endif
 
 #ifdef NTBUILD

lib/Common/Core/DelayLoadLibrary.cpp

@@ -317,4 +317,43 @@ NtdllLibrary::NTSTATUS NtdllLibrary::Close(_In_ HANDLE Handle)
 #endif
 }
 
+#ifndef DELAYLOAD_UNLOCKMEMORY
+extern "C"
+WINBASEAPI
+NtdllLibrary::NTSTATUS
+WINAPI
+NtUnlockVirtualMemory(
+    _In_ HANDLE ProcessHandle,
+    _Inout_ PVOID *BaseAddress,
+    _Inout_ PSIZE_T RegionSize,
+    _In_ ULONG MapType
+);
+#endif
+
+NtdllLibrary::NTSTATUS NtdllLibrary::UnlockVirtualMemory(
+    _In_ HANDLE ProcessHandle,
+    _Inout_ PVOID *BaseAddress,
+    _Inout_ PSIZE_T RegionSize,
+    _In_ ULONG MapType)
+{
+#ifdef DELAYLOAD_UNLOCKMEMORY
+    if (m_hModule)
+    {
+        if (unlock == nullptr)
+        {
+            unlock = (PFnNtUnlockVirtualMemory)GetFunction("NtUnlockVirtualMemory");
+            if (unlock == nullptr)
+            {
+                Assert(false);
+                return -1;
+            }
+        }
+        return unlock(ProcessHandle, BaseAddress, RegionSize, MapType);
+    }
+    return -1;
+#else
+    return NtUnlockVirtualMemory(ProcessHandle, BaseAddress, RegionSize, MapType);
+#endif
+}
+
 #endif // _WIN32

lib/Common/Core/DelayLoadLibrary.h

@@ -34,6 +34,7 @@ class NtdllLibrary : protected DelayLoadLibrary
 public:
     // needed for InitializeObjectAttributes
     static const ULONG OBJ_KERNEL_HANDLE = 0x00000200;
+    static const ULONG MAP_PROCESS = 1;
 
     typedef struct _UNICODE_STRING {
         USHORT Length;
@@ -105,6 +106,13 @@ class NtdllLibrary : protected DelayLoadLibrary
     typedef NTSTATUS(NTAPI *PFnNtClose)(_In_ HANDLE Handle);
     PFnNtClose close;
 
+    typedef NTSTATUS(NTAPI *PFnNtUnlockVirtualMemory)(
+        _In_ HANDLE ProcessHandle,
+        _Inout_ PVOID *BaseAddress,
+        _Inout_ PSIZE_T RegionSize,
+        _In_ ULONG MapType);
+    PFnNtUnlockVirtualMemory unlock;
+
 public:
     static NtdllLibrary* Instance;
 
@@ -117,7 +125,8 @@ class NtdllLibrary : protected DelayLoadLibrary
         createSection(NULL),
         mapViewOfSection(NULL),
         unmapViewOfSection(NULL),
-        close(NULL)
+        close(NULL),
+        unlock(nullptr)
     {
         this->EnsureFromSystemDirOnly();
     }
@@ -176,5 +185,12 @@ class NtdllLibrary : protected DelayLoadLibrary
     NTSTATUS Close(
         _In_ HANDLE Handle
     );
+
+    NTSTATUS UnlockVirtualMemory(
+        _In_ HANDLE ProcessHandle,
+        _Inout_ PVOID *BaseAddress,
+        _Inout_ PSIZE_T RegionSize,
+        _In_ ULONG MapType
+    );
 };
 #endif

lib/Common/Memory/SectionAllocWrapper.cpp

@@ -10,11 +10,21 @@
 #ifdef NTDDI_WIN10_RS2
 #if (NTDDI_VERSION >= NTDDI_WIN10_RS2)
 #define USEFILEMAP2 1
+#define USEVIRTUALUNLOCKEX 1
 #endif
 #endif
 
 namespace Memory
 {
+    
+void UnlockMemory(HANDLE process, LPVOID address, SIZE_T size)
+{
+#if USEVIRTUALUNLOCKEX
+    VirtualUnlockEx(process, address, size);
+#else
+    NtdllLibrary::Instance->UnlockVirtualMemory(process, &address, &size, NtdllLibrary::MAP_PROCESS);
+#endif
+}
 
 void CloseSectionHandle(HANDLE handle)
 {
@@ -593,12 +603,6 @@ SectionAllocWrapper::AllocPages(LPVOID requestAddress, size_t pageCount, DWORD a
             return nullptr;
         }
         address = requestAddress;
-
-        if ((allocationType & MEM_COMMIT) == MEM_COMMIT)
-        {
-            const DWORD allocProtectFlags = AutoSystemInfo::Data.IsCFGEnabled() ? PAGE_EXECUTE_RO_TARGETS_INVALID : PAGE_EXECUTE_READ;
-            address = VirtualAllocEx(this->process, address, dwSize, MEM_COMMIT, allocProtectFlags);
-        }
     }
 
     return address;
@@ -661,11 +665,7 @@ BOOL SectionAllocWrapper::Free(LPVOID lpAddress, size_t dwSize, DWORD dwFreeType
             ZeroMemory(localAddr, AutoSystemInfo::PageSize);
             FreeLocal(localAddr);
         }
-        DWORD oldFlags = NULL;
-        if (!VirtualProtectEx(this->process, lpAddress, dwSize, PAGE_NOACCESS, &oldFlags))
-        {
-            return FALSE;
-        }
+        UnlockMemory(this->process, lpAddress, dwSize);
     }
 
     return TRUE;
@@ -924,37 +924,15 @@ LPVOID PreReservedSectionAllocWrapper::AllocPages(LPVOID lpAddress, DECLSPEC_GUA
         AssertMsg(freeSegmentsBVIndex < PreReservedAllocationSegmentCount, "Invalid BitVector index calculation?");
         AssertMsg(dwSize % AutoSystemInfo::PageSize == 0, "COMMIT is managed at AutoSystemInfo::PageSize granularity");
 
-        char * allocatedAddress = nullptr;
-
-        if ((allocationType & MEM_COMMIT) != 0)
-        {
-#if defined(ENABLE_JIT_CLAMP)
-            AutoEnableDynamicCodeGen enableCodeGen;
-#endif
-
-            const DWORD allocProtectFlags = AutoSystemInfo::Data.IsCFGEnabled() ? PAGE_EXECUTE_RO_TARGETS_INVALID : PAGE_EXECUTE_READ;
-            allocatedAddress = (char *)VirtualAllocEx(this->process, addressToReserve, dwSize, MEM_COMMIT, allocProtectFlags);
-            if (allocatedAddress == nullptr)
-            {
-                MemoryOperationLastError::RecordLastError();
-            }
-        }
-        else
-        {
-            // Just return the uncommitted address if we didn't ask to commit it.
-            allocatedAddress = addressToReserve;
-        }
-
         // Keep track of the committed pages within the preReserved Memory Region
-        if (lpAddress == nullptr && allocatedAddress != nullptr)
+        if (lpAddress == nullptr)
         {
-            Assert(allocatedAddress == addressToReserve);
             Assert(requestedNumOfSegments != 0);
             freeSegments.ClearRange(freeSegmentsBVIndex, static_cast<uint>(requestedNumOfSegments));
-        }
+    }
 
-        PreReservedHeapTrace(_u("MEM_COMMIT: StartAddress: 0x%p of size: 0x%x * 0x%x bytes \n"), allocatedAddress, requestedNumOfSegments, AutoSystemInfo::Data.GetAllocationGranularityPageSize());
-        return allocatedAddress;
+        PreReservedHeapTrace(_u("MEM_COMMIT: StartAddress: 0x%p of size: 0x%x * 0x%x bytes \n"), addressToReserve, requestedNumOfSegments, AutoSystemInfo::Data.GetAllocationGranularityPageSize());
+        return addressToReserve;
     }
 }
 
@@ -989,11 +967,7 @@ PreReservedSectionAllocWrapper::Free(LPVOID lpAddress, size_t dwSize, DWORD dwFr
         FreeLocal(localAddr);
     }
 
-    DWORD oldFlags = NULL;
-    if(!VirtualProtectEx(this->process, lpAddress, dwSize, PAGE_NOACCESS, &oldFlags))
-    {
-        return FALSE;
-    }
+    UnlockMemory(this->process, lpAddress, dwSize);
 
     size_t requestedNumOfSegments = dwSize / AutoSystemInfo::Data.GetAllocationGranularityPageSize();
     Assert(requestedNumOfSegments <= MAXUINT32);