When unhooking, verify that the function wasn't changed

If for some reason the function was changed, e.g. if somebody else
placed a hook on top of ours, don't restore the original bytes over it,
which will remove the other hook and will likely cause a crash when that
other library will remove its hook. Instead, leak the allocated
trampoline and patch it such that it will jump straight to the original
code.

Author: m417z

Source/SlimDetours/Instruction.c

@@ -120,6 +120,20 @@ detour_gen_jmp_immediate(
     return pbCode + sizeof(INT32);
 }
 
+BOOL
+detour_is_jmp_immediate_to(
+    _In_ PBYTE pbCode,
+    _In_ PBYTE pbJmpVal)
+{
+    PBYTE pbJmpSrc = pbCode + 5;
+    if (*pbCode++ != 0xe9)   // jmp +imm32
+    {
+        return FALSE;
+    }
+    INT32 offset = *((INT32*)pbCode);
+    return offset == (INT32)(pbJmpVal - pbJmpSrc);
+}
+
 _Ret_notnull_
 PBYTE
 detour_gen_jmp_indirect(
@@ -139,6 +153,30 @@ detour_gen_jmp_indirect(
     return pbCode + sizeof(INT32);
 }
 
+BOOL
+detour_is_jmp_indirect_to(
+    _In_ PBYTE pbCode,
+    _In_ PBYTE* ppbJmpVal)
+{
+#if defined(_AMD64_)
+    PBYTE pbJmpSrc = pbCode + 6;
+#endif
+    if (*pbCode++ != 0xff)   // jmp [+imm32]
+    {
+        return FALSE;
+    }
+    if (*pbCode++ != 0x25)
+    {
+        return FALSE;
+    }
+    INT32 offset = *((INT32*)pbCode);
+#if defined(_AMD64_)
+    return offset == (INT32)((PBYTE)ppbJmpVal - pbJmpSrc);
+#else
+    return offset == (INT32)((PBYTE)ppbJmpVal);
+#endif
+}
+
 _Ret_notnull_
 PBYTE
 detour_gen_brk(
@@ -488,6 +526,37 @@ detour_gen_jmp_indirect(
     return pbCode;
 }
 
+BOOL
+detour_is_jmp_indirect_to(
+    _In_ PBYTE pbCode,
+    _In_ PULONG64 pbJmpVal)
+{
+    const struct ARM64_INDIRECT_JMP* pIndJmp;
+    union ARM64_INDIRECT_IMM jmpIndAddr;
+
+    jmpIndAddr.value = (((LONG64)pbJmpVal) & 0xFFFFFFFFFFFFF000) -
+        (((LONG64)pbCode) & 0xFFFFFFFFFFFFF000);
+
+    pIndJmp = (const struct ARM64_INDIRECT_JMP*)pbCode;
+
+    return pIndJmp->ardp.Rd == 17 &&
+        pIndJmp->ardp.immhi == (ULONG)jmpIndAddr.adrp_immhi &&
+        pIndJmp->ardp.iop == 0x10 &&
+        pIndJmp->ardp.immlo == (ULONG)jmpIndAddr.adrp_immlo &&
+        pIndJmp->ardp.op == 1 &&
+
+        pIndJmp->ldr.Rt == 17 &&
+        pIndJmp->ldr.Rn == 17 &&
+        pIndJmp->ldr.imm == (((ULONG64)pbJmpVal) & 0xFFF) / 8 &&
+        pIndJmp->ldr.opc == 1 &&
+        pIndJmp->ldr.iop1 == 1 &&
+        pIndJmp->ldr.V == 0 &&
+        pIndJmp->ldr.iop2 == 7 &&
+        pIndJmp->ldr.size == 3 &&
+
+        pIndJmp->br == 0xD61F0220;
+}
+
 _Ret_notnull_
 PBYTE
 detour_gen_jmp_immediate(

Source/SlimDetours/SlimDetours.inl

@@ -105,7 +105,8 @@ typedef struct _DETOUR_OPERATION DETOUR_OPERATION, *PDETOUR_OPERATION;
 struct _DETOUR_OPERATION
 {
     PDETOUR_OPERATION pNext;
-    BOOL fIsRemove;
+    BOOL fIsAdd : 1;
+    BOOL fIsRemove : 1;
     PBYTE* ppbPointer;
     PBYTE pbTarget;
     PDETOUR_TRAMPOLINE pTrampoline;
@@ -158,12 +159,22 @@ detour_gen_jmp_immediate(
     _In_ PBYTE pbCode,
     _In_ PBYTE pbJmpVal);
 
+BOOL
+detour_is_jmp_immediate_to(
+    _In_ PBYTE pbCode,
+    _In_ PBYTE pbJmpVal);
+
 _Ret_notnull_
 PBYTE
 detour_gen_jmp_indirect(
     _In_ PBYTE pbCode,
     _In_ PBYTE* ppbJmpVal);
 
+BOOL
+detour_is_jmp_indirect_to(
+    _In_ PBYTE pbCode,
+    _In_ PBYTE* ppbJmpVal);
+
 #elif defined(_ARM64_)
 
 _Ret_notnull_
@@ -179,6 +190,11 @@ detour_gen_jmp_indirect(
     _In_ PBYTE pbCode,
     _In_ PULONG64 pbJmpVal);
 
+BOOL
+detour_is_jmp_indirect_to(
+    _In_ PBYTE pbCode,
+    _In_ PULONG64 pbJmpVal);
+
 #endif
 
 _Ret_notnull_

Source/SlimDetours/Thread.c

@@ -153,9 +153,9 @@ detour_thread_update(
         return Status;
     }
 
-    for (o = PendingOperations; o != NULL; o = o->pNext)
+    bUpdateContext = FALSE;
+    for (o = PendingOperations; o != NULL && !bUpdateContext; o = o->pNext)
     {
-        bUpdateContext = FALSE;
         if (o->fIsRemove)
         {
             if (cxt.CONTEXT_PC >= (ULONG_PTR)o->pTrampoline->rbCode &&
@@ -172,7 +172,7 @@ detour_thread_update(
                 bUpdateContext = TRUE;
             }
 #endif
-        } else
+        } else if (o->fIsAdd)
         {
             if (cxt.CONTEXT_PC >= (ULONG_PTR)o->pbTarget &&
                 cxt.CONTEXT_PC < ((ULONG_PTR)o->pbTarget + o->pTrampoline->cbRestore))
@@ -182,11 +182,11 @@ detour_thread_update(
                 bUpdateContext = TRUE;
             }
         }
-        if (bUpdateContext)
-        {
-            Status = NtSetContextThread(ThreadHandle, &cxt);
-            break;
-        }
+    }
+
+    if (bUpdateContext)
+    {
+        Status = NtSetContextThread(ThreadHandle, &cxt);
     }
 
     return Status;

Source/SlimDetours/Transaction.c

@@ -111,7 +111,7 @@ SlimDetoursTransactionAbort(VOID)
         pMem = o->pbTarget;
         sMem = o->pTrampoline->cbRestore;
         NtProtectVirtualMemory(NtCurrentProcess(), &pMem, &sMem, o->dwPerm, &dwOld);
-        if (!o->fIsRemove)
+        if (o->fIsAdd)
         {
             detour_free_trampoline(o->pTrampoline);
             o->pTrampoline = NULL;
@@ -165,10 +165,29 @@ SlimDetoursTransactionCommit(VOID)
     {
         if (o->fIsRemove)
         {
-            RtlCopyMemory(o->pbTarget, o->pTrampoline->rbRestore, o->pTrampoline->cbRestore);
-            NtFlushInstructionCache(NtCurrentProcess(), o->pbTarget, o->pTrampoline->cbRestore);
+            // Check if the jmps still points where we expect, otherwise someone might have hooked us.
+            BOOL hookIsStillThere =
+#if defined(_X86_) || defined(_AMD64_)
+                detour_is_jmp_immediate_to(o->pbTarget, o->pTrampoline->rbCodeIn) &&
+                detour_is_jmp_indirect_to(o->pTrampoline->rbCodeIn, &o->pTrampoline->pbDetour);
+#elif defined(_ARM64_)
+                detour_is_jmp_indirect_to(o->pbTarget, (ULONG64*)&(o->pTrampoline->pbDetour));
+#endif
+
+            if (hookIsStillThere)
+            {
+                RtlCopyMemory(o->pbTarget, o->pTrampoline->rbRestore, o->pTrampoline->cbRestore);
+                NtFlushInstructionCache(NtCurrentProcess(), o->pbTarget, o->pTrampoline->cbRestore);
+            } else
+            {
+                // Don't remove in this case, put in bypass mode and leak trampoline.
+                o->fIsRemove = FALSE;
+                o->pTrampoline->pbDetour = o->pTrampoline->rbCode;
+                DETOUR_TRACE("detours: Leaked hook on pbTarget=%p due to external hooking\n", o->pbTarget);
+            }
+
             *o->ppbPointer = o->pbTarget;
-        } else
+        } else if (o->fIsAdd)
         {
             DETOUR_TRACE("detours: pbTramp =%p, pbRemain=%p, pbDetour=%p, cbRestore=%u\n",
                          o->pTrampoline,
@@ -457,6 +476,7 @@ SlimDetoursAttach(
                  pTrampoline->rbCode[8], pTrampoline->rbCode[9],
                  pTrampoline->rbCode[10], pTrampoline->rbCode[11]);
 
+    o->fIsAdd = TRUE;
     o->fIsRemove = FALSE;
     o->ppbPointer = (PBYTE*)ppPointer;
     o->pTrampoline = pTrampoline;
@@ -520,6 +540,7 @@ SlimDetoursDetach(
         goto fail;
     }
 
+    o->fIsAdd = FALSE;
     o->fIsRemove = TRUE;
     o->ppbPointer = (PBYTE*)ppPointer;
     o->pTrampoline = pTrampoline;