commit dda0385cc49240f8bd115476c870d61863741f4c Author: Arthur Edelstein arthuredelstein@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.