HookNtSetContextThread race condition and detection vector

[dauthleikr]

I am aware that detection vectors and race conditions have low priority due to nobody abusing them, but this one may be in actual use (not 100% sure yet, as understanding obfuscated code without breakpoints is not fun).

Race condition

TitanHide strips the ContextFlags in HookNtSetContextThread so that the debug registers will not be overwritten:

 Context->ContextFlags = OriginalContextFlags & ~0x10; //CONTEXT_DEBUG_REGISTERS ^ CONTEXT_AMD64/CONTEXT_i386

However TitanHide assumes that the ContextFlags cannot be set after the strip, and just passes the Context pointer to the actual function:

NTSTATUS ret = Undocumented::NtSetContextThread(ThreadHandle, Context);

This gives the caller plenty of time to try to un-strip the ContextFlags - consider the following proof of concept code:

void RaceFunction(CONTEXT* ptr, bool* run)
{
	while (*run)
	{
		ptr->ContextFlags = 0x10;
	}
}

void TryStealHardwareBreakpoints()
{
	CONTEXT ctx = {};
	ctx.ContextFlags = 0x10;
	if (!GetThreadContext(GetCurrentThread(), &ctx))
		return;
	// Dr0, ... are all 0 if this is run through TitanHide :)

	auto run = true;
	std::thread race(RaceFunction, &ctx, &run);
	SetThreadContext(GetCurrentThread(), &ctx);
	
	run = false;
	race.join();
}

Detection vector

An additional detection vector that I believe is being used in my target program is that GetThreadContext does not return a previously set hardware breakpoint when running under TitanHide. Proof of concept:

void DummyFunction() {}

bool HasTitanHide()
{
	const auto dummy_breakpoint = reinterpret_cast<DWORD64>(&DummyFunction);
	
	CONTEXT ctx = {};
	ctx.Dr0 = dummy_breakpoint;
	ctx.Dr7 = 1;
	ctx.ContextFlags = 0x10;

	if (!SetThreadContext(GetCurrentThread(), &ctx))
		return false;

	if (!GetThreadContext(GetCurrentThread(), &ctx))
		return false;

	if (ctx.Dr0 == dummy_breakpoint)
		return false;
	
	return true;
}

Possible fix

I understand that you guys are not trying to be super stealthy, but at least copying the Context before checking it and passing it on, to avoid the race condition, would be nice :)

[dauthleikr]

I found another issue with the current HookNtSetContextThread:
It does not check if the target of the SetThreadContext call is hidden, instead it checks if the process that called the function is hidden. This usually the same process, but it's possible that another process protects the debugged process by constantly removing it's hardware breakpoints.

Here's a proof of concept code that erases all hardware breakpoints from another process, even with TitanHide enabled:

DWORD pidToStealFrom = /* put some pid here */;
HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0);
if (snapshot != INVALID_HANDLE_VALUE)
{
    THREADENTRY32 te;
    te.dwSize = sizeof(te);
    if (Thread32First(snapshot, &te))
    {
	    do
	    {
		    if (te.dwSize >= FIELD_OFFSET(THREADENTRY32, th32OwnerProcessID) +
			    sizeof(te.th32OwnerProcessID) && te.th32OwnerProcessID == pidToStealFrom)
		    {
			    HANDLE threadHandle = OpenThread(THREAD_ALL_ACCESS, false, te.th32ThreadID);
			    if(threadHandle == NULL)
					continue;

				printf("Stealing hw bp's from Process 0x%04x Thread 0x%04x\n", te.th32OwnerProcessID, te.th32ThreadID);
				SuspendThread(threadHandle); // probably not needed
			    
				CONTEXT threadContext = {};
				threadContext.ContextFlags = 0x10;
				SetThreadContext(threadHandle, &threadContext);
			    
				ResumeThread(threadHandle); // probably not needed
		    }
		    te.dwSize = sizeof(te);
	    }
	    while (Thread32Next(snapshot, &te));
    }
    CloseHandle(snapshot);
}

The tricky thing here is that if you patch this issue, maybe debuggers such as x64dbg won't work either?

[mrexodia]

Heya, Generally TitanHide is fitting my needs, but I’ll be happy to review any PRs you might have. As you can probably tell by the code this was the first driver I ever wrote and it’s just a bunch of rootkits and other crap put together so don’t worry about the quality of the code ^^

…

On Thu, 27 Feb 2020 at 19:54, dauthleikr ***@***.***> wrote: I found another issue with the current HookNtSetContextThread: It does not check if the target of the SetThreadContext call is hidden, instead it checks if the process that called the function is hidden. This usually the same process, but it's possible that another process protects the debugged process by constantly removing it's hardware breakpoints. Here's a proof of concept code that erases all hardware breakpoints from another process, even if with TitanHide enabled: DWORD pidToStealFrom = /* put some pid here */; HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPTHREAD, 0);if (snapshot != INVALID_HANDLE_VALUE) { THREADENTRY32 te; te.dwSize = sizeof(te); if (Thread32First(snapshot, &te)) { do { if (te.dwSize >= FIELD_OFFSET(THREADENTRY32, th32OwnerProcessID) + sizeof(te.th32OwnerProcessID) && te.th32OwnerProcessID == pidToStealFrom) { HANDLE threadHandle = OpenThread(THREAD_ALL_ACCESS, false, te.th32ThreadID); if(threadHandle == NULL) continue; printf("Stealing hw bp's from Process 0x%04x Thread 0x%04x\n", te.th32OwnerProcessID, te.th32ThreadID); SuspendThread(threadHandle); // probably not needed CONTEXT threadContext = {}; threadContext.ContextFlags = 0x10; SetThreadContext(threadHandle, &threadContext); ResumeThread(threadHandle); // probably not needed } te.dwSize = sizeof(te); } while (Thread32Next(snapshot, &te)); } CloseHandle(snapshot); } If you guys want I can submit a PR for all these issues, but I have no experience writing drivers so I might produce bugs/vulnerabilities ... — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#44?email_source=notifications&email_token=AASYFGNMO76BN4LWM3L322DRFAD65A5CNFSM4K4N3TW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENFQQQQ#issuecomment-592119874>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AASYFGLY57LWOCR7CU2RWFLRFAD65ANCNFSM4K4N3TWQ> .

[dauthleikr]

Closing this for now, a proper implementation is not trivial, and I doubt anybody really abuses this specific vector

[mrexodia]

I think the vector is still there so I'll keep the issue open :)