NtSystemDebugControl Incomplete

[stevemk14ebr]

The range of Command values need to be widened to include the valid values above the Triage command. It should be:

if (((Command - SysDbgGetTriageDump) & 0xFFFFFFF7) != 0) {
        return STATUS_DEBUGGER_INACTIVE;
    } 

For values not in this range, they have a check done on them to see if the process has SeDebugPriv, which we want to always fail, which should return STATUS_ACCESS_DENIED. EXCEPT, for code 0x26 which should be forwarded to the original call.

[Mattiwatti]

I checked out a recent kernel and it does do the AND trick, but I'm pretty sure this is just the way the compiler optimized the code. If you work out the values it comes down to the same as
if (Command != SysDbgGetTriageDump /*=29*/ && Command != SysDbgGetLiveKernelDump /*=37*/)

I was going to add SysDbgGetLiveKernelDump to the if statement, but then I saw it was already added in 7e2993d. So this was indeed a bug at one point but not anymore.

Re: SeDebugPrivilege: why do you say we want this check to fail? I don't think I agree. Consider what happens if a process legitimately has debug privileges at some point regardless of whether it's being debugged, e.g.:

if (NT_SUCCESS(RtlAdjustPrivilege(SE_DEBUG_PRIVILEGE, TRUE, FALSE, &WasEnabled)))
{
    Status = NtSystemDebugControl(SysDbgGetTriageDump, &SysDbgTriageDumpRequest,
    sizeof(SYSDBG_TRIAGE_DUMP), Buffer, BufferLength,  &ReturnLength);
    if (Status == STATUS_ACCESS_DENIED)
    {
        // TitanHide detected!
    }
}

We used to have this behaviour in ScyllaHide in NtSetDebugFilterState but it was removed due to this. I guess there is something to be said for doing nothing but still returning success, but I don't really think letting the syscall go through will do much harm. If the intent is to detect TitanHide, it's still going to be a lot more work for someone to go through the triage dump to find TitanHide in it than it is to check if the output buffer was touched.