Skip to content

Replace internal CreateThread calls with an internal implementation to avoid monitor detection by NtQueryInformationThread using ThreadQuerySetWin32StartAddress #95

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: capemon
Choose a base branch
from
Open

Replace internal CreateThread calls with an internal implementation to avoid monitor detection by NtQueryInformationThread using ThreadQuerySetWin32StartAddress #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions misc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,47 @@ DWORD tid_from_thread_handle(HANDLE thread_handle)
return (DWORD)(ULONG_PTR)cid.UniqueThread;
}

VOID CALLBACK thread_callback(PVOID context_param, BOOLEAN timeout)
{
THREAD_CONTEXT *context = (THREAD_CONTEXT*)context_param;

if (context->thread_id)
*context->thread_id = GetCurrentThreadId();
Comment on lines +806 to +807
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The thread ID is retrieved within the thread_callback function using GetCurrentThreadId(). This ID is then stored in the context->thread_id pointer. However, the thread might terminate before the ID is retrieved, leading to a potential race condition or incorrect thread ID being stored. Consider using a synchronization mechanism to ensure that the thread ID is retrieved before the thread terminates, or use a different approach to retrieve the thread ID.

Suggested change
if (context->thread_id)
*context->thread_id = GetCurrentThreadId();
DWORD threadId = GetCurrentThreadId(); // Get the current thread ID
if (context->thread_id) {
*context->thread_id = threadId; // Store the thread ID
}


context->start_routine(context->parameter);

SetEvent(context->event);
CloseHandle(context->event);
free(context);
Comment on lines +811 to +813
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Consider adding error handling around SetEvent and CloseHandle to ensure that these resources are properly managed. Although these functions are unlikely to fail, it's good practice to check for errors, especially in a system-level context. It would also be good to add a comment explaining why these functions are called.

Suggested change
SetEvent(context->event);
CloseHandle(context->event);
free(context);
if (!SetEvent(context->event)) { // Signal that the thread has completed
// Log the error, but continue cleanup
DWORD err = GetLastError();
DebugOutput("SetEvent failed with error: %d\n", err);
}
if (!CloseHandle(context->event)) { // Close the event handle
// Log the error, but continue cleanup
DWORD err = GetLastError();
DebugOutput("CloseHandle failed with error: %d\n", err);
}

}

HANDLE our_createthread(PTHREAD_START_ROUTINE lpStartAddress, LPVOID lpParameter, DWORD *lpThreadId)
{
HANDLE timer = NULL;

THREAD_CONTEXT *context = (THREAD_CONTEXT*)calloc(1, sizeof(THREAD_CONTEXT));
if (!context)
return NULL;

context->start_routine = lpStartAddress;
context->parameter = lpParameter;
context->thread_id = lpThreadId;

context->event = CreateEvent(NULL, TRUE, FALSE, NULL);
if (!context->event) {
free(context);
return NULL;
}

if (!CreateTimerQueueTimer(&timer, NULL, thread_callback, context, 0, 0, WT_EXECUTEDEFAULT)) {
CloseHandle(context->event);
free(context);
return NULL;
Comment on lines +834 to +837
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The CreateTimerQueueTimer function creates a timer. The timer handle is not explicitly managed. It's important to ensure that the timer is properly cleaned up to avoid resource leaks. Consider using DeleteTimerQueueTimer to remove the timer from the queue and release associated resources. It's also important to add a comment explaining why these functions are called.

Suggested change
if (!CreateTimerQueueTimer(&timer, NULL, thread_callback, context, 0, 0, WT_EXECUTEDEFAULT)) {
CloseHandle(context->event);
free(context);
return NULL;
CloseHandle(context->event);
free(context);
return NULL;
}
// Timer created successfully, but ensure it's cleaned up if the function fails later
context->timer = timer; // Store the timer handle in the context

}

return context->event;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The function returns the event handle, but the actual thread execution is asynchronous via a timer. If the timer creation fails, the event handle is closed and memory is freed. However, if the timer creation succeeds, the caller receives the event handle but has no direct way to manage the timer's lifecycle. This could lead to confusion and potential resource leaks if the caller expects a traditional thread handle. Consider returning the timer handle instead of the event handle, or providing a separate function to manage the timer's lifecycle.

Suggested change
return context->event;
// Returning the timer handle instead of the event handle
return timer;

}

DWORD our_getprocessid(HANDLE Process)
{
DWORD ret;
Expand Down
9 changes: 9 additions & 0 deletions misc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,14 @@ typedef SIZE_T (WINAPI *_RtlCompareMemory)(
_In_ SIZE_T Length
);

typedef struct _THREAD_CONTEXT
{
PTHREAD_START_ROUTINE start_routine;
LPVOID parameter;
HANDLE event;
LPDWORD thread_id;
} THREAD_CONTEXT;

_NtSetInformationProcess pNtSetInformationProcess;
_NtMapViewOfSection pNtMapViewOfSection;
_NtUnmapViewOfSection pNtUnmapViewOfSection;
Expand Down Expand Up @@ -195,6 +203,7 @@ struct dll_range {
DWORD get_pid_by_tid(DWORD tid);

DWORD our_getprocessid(HANDLE Process);
HANDLE our_createthread(PTHREAD_START_ROUTINE lpStartAddress, LPVOID lpParameter, DWORD *lpThreadId);
BOOL is_in_dll_range(ULONG_PTR addr);
void add_all_dlls_to_dll_ranges(void);
void add_dll_range(ULONG_PTR start, ULONG_PTR end);
Expand Down
17 changes: 6 additions & 11 deletions unhook.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,15 +254,12 @@ DWORD g_unhook_watcher_thread_id;

int unhook_init_detection()
{
g_unhook_thread_handle =
CreateThread(NULL, 0, &_unhook_detect_thread, NULL, 0, &g_unhook_detect_thread_id);
g_unhook_thread_handle = our_createthread(&_unhook_detect_thread, NULL, &g_unhook_detect_thread_id);

g_watcher_thread_handle =
CreateThread(NULL, 0, &_unhook_watch_thread, NULL, 0, &g_unhook_watcher_thread_id);
g_watcher_thread_handle = our_createthread(&_unhook_watch_thread, NULL, &g_unhook_watcher_thread_id);

if(g_unhook_thread_handle != NULL && g_watcher_thread_handle != NULL) {
if (g_unhook_thread_handle != NULL && g_watcher_thread_handle != NULL)
return 0;
}

pipe("CRITICAL:Error initializing unhook detection threads!");
return -1;
Expand Down Expand Up @@ -338,8 +335,7 @@ int terminate_event_init()
sa.lpSecurityDescriptor = &sd;
g_terminate_event_handle = CreateEventA(&sa, FALSE, FALSE, g_config.terminate_event_name);

g_terminate_event_thread_handle =
CreateThread(NULL, 0, &_terminate_event_thread, NULL, 0, &g_terminate_event_thread_id);
g_terminate_event_thread_handle = our_createthread(&_terminate_event_thread, NULL, &g_terminate_event_thread_id);

if (g_terminate_event_handle != NULL && g_terminate_event_thread_handle != NULL)
return 0;
Expand Down Expand Up @@ -397,8 +393,7 @@ int procname_watch_init()
InitialProcessPath.Buffer = (PWSTR)calloc(mod->FullDllName.MaximumLength, 1);
memcpy(InitialProcessPath.Buffer, mod->FullDllName.Buffer, InitialProcessPath.Length);

g_procname_watch_thread_handle =
CreateThread(NULL, 0, &_procname_watch_thread, NULL, 0, &g_procname_watcher_thread_id);
g_procname_watch_thread_handle = our_createthread(&_procname_watch_thread, NULL, &g_procname_watcher_thread_id);

if (g_procname_watch_thread_handle != NULL)
return 0;
Expand Down Expand Up @@ -482,7 +477,7 @@ int init_watchdog()

DuplicateHandle(GetCurrentProcess(), GetCurrentThread(), GetCurrentProcess(), &mainthreadhandle, THREAD_ALL_ACCESS, FALSE, 0);

CreateThread(NULL, 0, &_watchdog_thread, mainthreadhandle, 0, &g_watchdog_thread_id);
our_createthread(&_watchdog_thread, mainthreadhandle, &g_watchdog_thread_id);

return 0;
}
Expand Down