trustedsec / cs-remote-ops-bof Goto Github PK

    for (i = 0; i < handleInfo->Count; i++) {
        SYSTEM_HANDLE_ENTRY handle = handleInfo->Handle[i];
        UNICODE_STRING objectName;
        ULONG returnLength = 0;

...
-        if (processHandle) {    <<< because processHandle is valid, this code does actually close the dup'd handles, but I think the code intents to check dupHandle
+         if (dupHandle) {
            KERNEL32$CloseHandle(dupHandle); 
            dupHandle = NULL;
        }

 killit_end:
    if (handleInfo) {
        intFree(handleInfo);
        handleInfo = NULL;
    }

    if (processHandle) {
        KERNEL32$CloseHandle(processHandle);
!        processHandle = NULL;  <<< processHandle set to NULL
    }

    if (objectTypeInfo) {
        intFree(objectTypeInfo);
        objectTypeInfo = NULL;
    }

    if (objectNameInfo) {
        intFree(objectNameInfo);
        objectNameInfo = NULL;
    }

-    if (processHandle) {   <<< processHandle is guaranteed to be NULL here so this branch never executes
+    if (dupHandle) {
        KERNEL32$CloseHandle(dupHandle);
        dupHandle = NULL;
    }

delete_regkey_end:
	if(RemoteKey)
	{
-		ADVAPI32$RegCloseKey(rootkey);
+		ADVAPI32$RegCloseKey(RemoteKey);
-		rootkey = NULL;
+		RemoteKey = NULL;
	}

	if(rootkey)
	{
		ADVAPI32$RegCloseKey(rootkey);
		rootkey = NULL;
	}
	
	if(targetkey)
	{
		ADVAPI32$RegCloseKey(targetkey);
		targetkey = NULL;
	}

VOID go( 
	IN PCHAR Buffer, 
	IN ULONG Length 
) 
{

...
	HANDLE currentTokenHandle = NULL;
!	BOOL getCurrentToken = ADVAPI32$OpenProcessToken(KERNEL32$GetCurrentProcess(), TOKEN_ADJUST_PRIVILEGES, &currentTokenHandle);   <<< must close currentTokenHandle
...
	if(getCurrentToken)
	{
		if (!SetPrivilege(currentTokenHandle, "SeDebugPrivilege", FALSE))
		{
			internal_printf("[+] SeDebugPrivilege Disabled!\n");
		}
+		KERNEL32$CloseHandle(currentTokenHandle);
	}
...
};

DWORD createTask(const wchar_t * server, wchar_t * taskpath, const wchar_t* xmldef, int mode, BOOL force)
{
	HRESULT hr = S_OK;
	VARIANT Vserver;
	VARIANT VNull;
	VARIANT Vsddl;
	VARIANT Vthisuser;
@@ NOTE: VARIANTs are uninitialized at this point @@
...


	// Initialize COM
	hr = OLE32$CoInitializeEx(NULL, COINIT_APARTMENTTHREADED);
	if(FAILED(hr))
	{
		internal_printf("Failed to initialize COM (%lX)\n", hr);
!		goto createTask_end;  <<< jumps to cleanup before initializing variants
	}

	// Create System user string
	BSTRsystem = OLEAUT32$SysAllocString(USER_SYSTEM_STRING);
	if (NULL == BSTRsystem)
	{
		hr = ERROR_OUTOFMEMORY;
		internal_printf("SysAllocString failed (%lX)\n", hr);
!		goto createTask_end;     <<< jumps to cleanup before initializing variants
	}

	// Initialize variants
	OLEAUT32$VariantInit(&Vserver);
	OLEAUT32$VariantInit(&VNull);
	OLEAUT32$VariantInit(&Vsddl);
	OLEAUT32$VariantInit(&Vthisuser); // we don't clear this because we free both possible OLE strings
@@ NOTE: NOW VARIANTs are initialized @@
...


createTask_end:
...

!	OLEAUT32$VariantClear(&Vsddl);   <<<  security bug from calling VariantClear on uninitialized variant 💥
!	OLEAUT32$VariantClear(&Vserver); <<<  security bug from calling VariantClear on uninitialized variant 💥
 	//OLEAUT32$VariantInit(&Vthisuser); // we don't clear this because we free both possible OLE strings
	OLE32$CoUninitialize();

	return (DWORD)hr;
}