Bug 1803334 - Skip execution of MovPushRet and PushRet in TestDllInterceptor if Intel CET is active. r=handyman

Bug 1596930 added support for detouring a pattern of code used by eScan
Internet Security Suite. The patch also added tests to make sure
that we correctly detour this pattern.

The pattern involves a PUSH instruction followed by a RET instruction.
This pattern is forbidden by Intel CET, which enforces at RET time that
we always return to an address that was pushed on the stack by a
prior CALL instruction. Executing the pattern thus crashes if Intel CET
is active.

If CET is active, we must thus skip the execution part of the test, or
the test crashes. We will still check that our detouring code
recognized the pattern and detoured it, but we will not run the detoured
pattern anymore under active Intel CET.

Differential Revision: https://phabricator.services.mozilla.com/D163468

mozglue/misc/WindowsProcessMitigations.cpp

@@ -88,4 +88,20 @@ MFBT_API bool IsEafPlusEnabled() {
   return polInfo.EnableExportAddressFilterPlus;
 }
 
+MFBT_API bool IsUserShadowStackEnabled() {
+  auto pGetProcessMitigationPolicy = FetchGetProcessMitigationPolicyFunc();
+  if (!pGetProcessMitigationPolicy) {
+    return false;
+  }
+
+  PROCESS_MITIGATION_USER_SHADOW_STACK_POLICY polInfo;
+  if (!pGetProcessMitigationPolicy(::GetCurrentProcess(),
+                                   ProcessUserShadowStackPolicy, &polInfo,
+                                   sizeof(polInfo))) {
+    return false;
+  }
+
+  return polInfo.EnableUserShadowStack;
+}
+
 }  // namespace mozilla

mozglue/misc/WindowsProcessMitigations.h

@@ -15,6 +15,7 @@ MFBT_API bool IsWin32kLockedDown();
 MFBT_API void SetWin32kLockedDownInPolicy();
 MFBT_API bool IsDynamicCodeDisabled();
 MFBT_API bool IsEafPlusEnabled();
+MFBT_API bool IsUserShadowStackEnabled();
 
 }  // namespace mozilla
 

toolkit/xre/dllservices/tests/TestDllInterceptor.cpp

@@ -20,6 +20,7 @@
 #include "AssemblyPayloads.h"
 #include "mozilla/DynamicallyLinkedFunctionPtr.h"
 #include "mozilla/UniquePtr.h"
+#include "mozilla/WindowsProcessMitigations.h"
 #include "nsWindowsDllInterceptor.h"
 #include "nsWindowsHelpers.h"
 
@@ -807,10 +808,13 @@ struct TestCase {
   const char* mFunctionName;
   uintptr_t mExpectedStub;
   bool mPatchedOnce;
-  explicit TestCase(const char* aFunctionName, uintptr_t aExpectedStub)
+  bool mSkipExec;
+  explicit TestCase(const char* aFunctionName, uintptr_t aExpectedStub,
+                    bool aSkipExec = false)
       : mFunctionName(aFunctionName),
         mExpectedStub(aExpectedStub),
-        mPatchedOnce(false) {}
+        mPatchedOnce(false),
+        mSkipExec(aSkipExec) {}
 } g_AssemblyTestCases[] = {
 #if defined(__clang__)
 // We disable these testcases because the code coverage instrumentation injects
@@ -819,7 +823,8 @@ struct TestCase {
 #    if defined(_M_X64)
     // Since we have PatchIfTargetIsRecognizedTrampoline for x64, we expect the
     // original jump destination is returned as a stub.
-    TestCase("MovPushRet", JumpDestination),
+    TestCase("MovPushRet", JumpDestination,
+             mozilla::IsUserShadowStackEnabled()),
     TestCase("MovRaxJump", JumpDestination),
     TestCase("DoubleJump", JumpDestination),
 
@@ -831,7 +836,8 @@ struct TestCase {
     TestCase("MovImm64", NoStubAddressCheck),
 #    elif defined(_M_IX86)
     // Skip the stub address check as we always generate a trampoline for x86.
-    TestCase("PushRet", NoStubAddressCheck),
+    TestCase("PushRet", NoStubAddressCheck,
+             mozilla::IsUserShadowStackEnabled()),
     TestCase("MovEaxJump", NoStubAddressCheck),
     TestCase("DoubleJump", NoStubAddressCheck),
     TestCase("Opcode83", NoStubAddressCheck),
@@ -912,18 +918,25 @@ bool TestAssemblyFunctions() {
       return false;
     }
 
-    patched_func_called = false;
-
-    auto originalFunction = reinterpret_cast<void (*)()>(
-        GetProcAddress(GetModuleHandleW(nullptr), testCase.mFunctionName));
-    originalFunction();
-
-    if (!patched_func_called) {
+    if (testCase.mSkipExec) {
       printf(
-          "TEST-FAILED | WindowsDllInterceptor | "
-          "Hook from %s was not called\n",
+          "TEST-SKIPPED | WindowsDllInterceptor | "
+          "Will not attempt to execute patched %s.\n",
           testCase.mFunctionName);
-      return false;
+    } else {
+      patched_func_called = false;
+
+      auto originalFunction = reinterpret_cast<void (*)()>(
+          GetProcAddress(GetModuleHandleW(nullptr), testCase.mFunctionName));
+      originalFunction();
+
+      if (!patched_func_called) {
+        printf(
+            "TEST-FAILED | WindowsDllInterceptor | "
+            "Hook from %s was not called\n",
+            testCase.mFunctionName);
+        return false;
+      }
     }
 
     printf("TEST-PASS | WindowsDllInterceptor | %s\n", testCase.mFunctionName);