Merge pull request #16 from shocoman/memory-read-safe-page-bug

Fix 'MemoryReadSafe' not restoring original memory protection correctly
x64dbg · Oct 28, 2023 · 49f5978 · 49f5978
2 parents 5484a49 + 4bac132
commit 49f5978
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 2 deletions.
diff --git a/TitanEngine/Global.Engine.Threading.h b/TitanEngine/Global.Engine.Threading.h
@@ -10,6 +10,7 @@
 enum CriticalSectionLock
 {
     LockBreakPointBuffer,
+    LockMemoryProtection,
     LockLast
 };
 

diff --git a/TitanEngine/TitanEngine.Debugger.Memory.cpp b/TitanEngine/TitanEngine.Debugger.Memory.cpp
@@ -350,13 +350,33 @@ __declspec(dllexport) bool TITCALL MemoryReadSafe(HANDLE hProcess, LPVOID lpBase
 
     if(!ReadProcessMemory(hProcess, lpBaseAddress, lpBuffer, nSize, pNumBytes))
     {
+        CriticalSectionLocker memProtectLock(LockMemoryProtection);
+
+        // try to temporarily change the page protections to PAGE_EXECUTE_READ
+        std::vector<MEMORY_BASIC_INFORMATION> memRegions;
+        MEMORY_BASIC_INFORMATION memInfo;
+        ULONG_PTR endAddr = (ULONG_PTR)lpBaseAddress + nSize;
+        for(ULONG_PTR page = ALIGN_DOWN_BY(lpBaseAddress, TITANENGINE_PAGESIZE); page < endAddr; page += memInfo.RegionSize)
+        {
+            if(0 == VirtualQueryEx(hProcess, (LPCVOID)page, &memInfo, sizeof memInfo))
+                break; // failure ('VirtualProtectEx' will fail too)
+            memRegions.push_back(memInfo);
+        }
+
         if(VirtualProtectEx(hProcess, lpBaseAddress, nSize, PAGE_EXECUTE_READ, &dwProtect))
         {
             if(ReadProcessMemory(hProcess, lpBaseAddress, lpBuffer, nSize, pNumBytes))
             {
                 retValue = true;
             }
-            VirtualProtectEx(hProcess, lpBaseAddress, nSize, dwProtect, &dwProtect);
+
+            for(const auto & info : memRegions)
+            {
+                ULONG_PTR size = info.RegionSize;
+                if(endAddr < (ULONG_PTR)info.BaseAddress + info.RegionSize)
+                    size = endAddr - (ULONG_PTR)info.BaseAddress;
+                VirtualProtectEx(hProcess, info.BaseAddress, size, info.Protect, &dwProtect);
+            }
         }
     }
     else
@@ -402,13 +422,33 @@ __declspec(dllexport) bool TITCALL MemoryWriteSafe(HANDLE hProcess, LPVOID lpBas
 
     if(!WriteProcessMemory(hProcess, lpBaseAddress, lpBuffer, nSize, pNumBytes) || *pNumBytes < nSize)
     {
+        CriticalSectionLocker memProtectLock(LockMemoryProtection);
+
+        // try to temporarily change the page protections to PAGE_EXECUTE_READWRITE
+        std::vector<MEMORY_BASIC_INFORMATION> memRegions;
+        MEMORY_BASIC_INFORMATION memInfo;
+        ULONG_PTR endAddr = (ULONG_PTR)lpBaseAddress + nSize;
+        for(ULONG_PTR page = ALIGN_DOWN_BY(lpBaseAddress, TITANENGINE_PAGESIZE); page < endAddr; page += memInfo.RegionSize)
+        {
+            if(0 == VirtualQueryEx(hProcess, (LPCVOID)page, &memInfo, sizeof memInfo))
+                break; // failure
+            memRegions.push_back(memInfo);
+        }
+
         if(VirtualProtectEx(hProcess, lpBaseAddress, nSize, PAGE_EXECUTE_READWRITE, &dwProtect))
         {
             if(WriteProcessMemory(hProcess, lpBaseAddress, lpBuffer, nSize, pNumBytes))
             {
                 retValue = true;
             }
-            VirtualProtectEx(hProcess, lpBaseAddress, nSize, dwProtect, &dwProtect);
+
+            for(const auto & info : memRegions)
+            {
+                ULONG_PTR size = info.RegionSize;
+                if(endAddr < (ULONG_PTR)info.BaseAddress + info.RegionSize)
+                    size = endAddr - (ULONG_PTR)info.BaseAddress;
+                VirtualProtectEx(hProcess, info.BaseAddress, size, info.Protect, &dwProtect);
+            }
         }
     }
     else