[tbb-commits] [tor-browser/tor-browser-52.2.0esr-7.5-1] Bug 1344034 - Auto-enforce W^X for WindowsDllInterceptor hook pages. r=dmajor

[gk at torproject.org]

commit dda0385cc49240f8bd115476c870d61863741f4c
Author: Arthur Edelstein <arthuredelstein at gmail.com>
Date:   Thu Jun 8 09:25:54 2017 -0700

    Bug 1344034 - Auto-enforce W^X for WindowsDllInterceptor hook pages. r=dmajor
---
 xpcom/build/nsWindowsDllInterceptor.h | 35 ++++++++++-------------------------
 1 file changed, 10 insertions(+), 25 deletions(-)

diff --git a/xpcom/build/nsWindowsDllInterceptor.h b/xpcom/build/nsWindowsDllInterceptor.h
index 21a8f05..397fabc 100644
--- a/xpcom/build/nsWindowsDllInterceptor.h
+++ b/xpcom/build/nsWindowsDllInterceptor.h
@@ -93,6 +93,9 @@ public:
   {
     mSuccess = !!VirtualProtectEx(GetCurrentProcess(), mFunc, mSize,
                                   mNewProtect, &mOldProtect);
+    if (!mSuccess) {
+      // printf("VirtualProtectEx failed! %d\n", GetLastError());
+    }
     return mSuccess;
   }
 
@@ -132,7 +135,6 @@ public:
       // Ensure we can write to the code.
       AutoVirtualProtect protect(fn, 2, PAGE_EXECUTE_READWRITE);
       if (!protect.Protect()) {
-        // printf("VirtualProtectEx failed! %d\n", GetLastError());
         continue;
       }
 
@@ -268,7 +270,6 @@ public:
     AutoVirtualProtect protectBefore(fn - 5, 5, PAGE_EXECUTE_READWRITE);
     AutoVirtualProtect protectAfter(fn, 2, PAGE_EXECUTE_READWRITE);
     if (!protectBefore.Protect() || !protectAfter.Protect()) {
-      //printf ("VirtualProtectEx failed! %d\n", GetLastError());
       return false;
     }
 
@@ -377,7 +378,6 @@ public:
       // ensure we can modify the original code
       AutoVirtualProtect protect(origBytes, nBytes, PAGE_EXECUTE_READWRITE);
       if (!protect.Protect()) {
-        //printf ("VirtualProtectEx failed! %d\n", GetLastError());
         continue;
       }
 
@@ -417,7 +417,7 @@ public:
     mHookPage = (byteptr_t)VirtualAllocEx(GetCurrentProcess(), nullptr,
                                           mMaxHooks * kHookSize,
                                           MEM_COMMIT | MEM_RESERVE,
-                                          PAGE_EXECUTE_READWRITE);
+                                          PAGE_EXECUTE_READ);
     if (!mHookPage) {
       mModule = 0;
       return;
@@ -426,19 +426,6 @@ public:
 
   bool Initialized() { return !!mModule; }
 
-  void LockHooks()
-  {
-    if (!mModule) {
-      return;
-    }
-
-    DWORD op;
-    VirtualProtectEx(GetCurrentProcess(), mHookPage, mMaxHooks * kHookSize,
-                     PAGE_EXECUTE_READ, &op);
-
-    mModule = 0;
-  }
-
   bool AddHook(const char* aName, intptr_t aHookDest, void** aOrigFunc)
   {
     if (!mModule) {
@@ -670,6 +657,12 @@ protected:
   {
     *aOutTramp = nullptr;
 
+    AutoVirtualProtect protectHookPage(mHookPage, mMaxHooks * kHookSize,
+                                       PAGE_EXECUTE_READWRITE);
+    if (!protectHookPage.Protect()) {
+      return;
+    }
+
     byteptr_t tramp = FindTrampolineSpace();
     if (!tramp) {
       return;
@@ -1005,7 +998,6 @@ protected:
     // ensure we can modify the original code
     AutoVirtualProtect protect(aOrigFunction, nBytes, PAGE_EXECUTE_READWRITE);
     if (!protect.Protect()) {
-      //printf ("VirtualProtectEx failed! %d\n", GetLastError());
       return;
     }
 
@@ -1091,13 +1083,6 @@ public:
     // not need it.
   }
 
-  void LockHooks()
-  {
-    if (mDetourPatcher.Initialized()) {
-      mDetourPatcher.LockHooks();
-    }
-  }
-
   bool AddHook(const char* aName, intptr_t aHookDest, void** aOrigFunc)
   {
     // Use a nop space patch if possible, otherwise fall back to a detour.