WMI hooks#96
Conversation
Initial commit for WMI hooks
There was a problem hiding this comment.
Hello @kevoreilly, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello team, gemini-code-assist here to provide a summary of this pull request titled "WMI hooks". Based on the title and the changes presented in the patches, the primary goal of this PR is to introduce hooking capabilities for Windows Management Instrumentation (WMI) API calls. This involves adding new hooks for key WMI functions, integrating them into the existing hooking framework, and adding necessary YARA signatures to locate these functions dynamically. Additionally, the PR includes updates to anti-analysis spoofing mechanisms, consolidating configuration values and applying them to existing file system and system information hooks, and adds support for logging VARIANT types, which are commonly used in COM/WMI interfaces.
Highlights
- WMI Hooking: Introduces new hooks for several WMI API functions found in
fastprox.dll, includingExecQuery,ExecMethod,GetObject, and their asynchronous counterparts, as well asWMI_Get. These hooks are designed to log WMI activity. - YARA Signature Integration: Adds YARA signatures for the newly hooked WMI functions to
YaraHarness.c, allowing the hooking engine to locate these functions dynamically in target processes. - Anti-Analysis Spoofing Refinement: Adds new configuration constants in
config.hfor spoofing system details like disk size, CPU core count, GPU RAM/name, and total/reserved RAM. Existing hooks forGetDiskFreeSpaceEx,GetDiskFreeSpace,GetSystemInfo, andNtQuerySystemInformationare updated to use these new constants, centralizing the spoofing values. - VARIANT Type Logging: Adds a new helper function
log_variantand a new format specifier ('n') to the logging mechanism (log.c,log.h) to properly handle and log values stored inVARIANTstructures, which is essential for capturing details from COM/WMI calls.
Changelog
Click here to see the changelog
- CAPE/CAPE.c
- Added WMI function names (
WMI_ExecQuery,WMI_ExecMethod,WMI_ExecQueryAsync,WMI_ExecMethodAsync,WMI_GetObject,WMI_GetObjectAsync) to theYaraFunctionsarray (lines 701-706). - Changed debug print format specifier for addresses in
GetAddressByYarafrom%xto%p(line 388).
- Added WMI function names (
- CAPE/YaraHarness.c
- Added YARA rules (byte signatures) for WMI functions (
WMI_ExecQuery,WMI_ExecMethod,WMI_ExecQueryAsync,WMI_ExecMethodAsync,WMI_GetObject,WMI_GetObjectAsync) to theInternalYarastring (lines 54-70).
- Added YARA rules (byte signatures) for WMI functions (
- capemon.vcxproj
- Added
hook_wmi.cto the list of compiled source files (line 269).
- Added
- capemon.vcxproj.filters
- Added
hook_wmi.cto the 'Source Files' filter group (lines 306-308).
- Added
- config.h
- Added new
#defineconstants for anti-analysis spoofing values (disk size, recovery partition size, GPU RAM/name, RAM size/reserved, CPU core count) (lines 26-36). - Removed trailing newline at the end of the file (line 317).
- Added new
- hook_file.c
- Updated
GetDiskFreeSpaceExAhook to useSPOOFED_DISK_SIZEandRECOVERY_PARTITION_SIZEconstants for spoofing total disk size (line 1355). - Updated
GetDiskFreeSpaceExWhook to useSPOOFED_DISK_SIZEandRECOVERY_PARTITION_SIZEconstants for spoofing total disk size (line 1370). - Updated
GetDiskFreeSpaceAhook to useSPOOFED_DISK_SIZEandRECOVERY_PARTITION_SIZEconstants for spoofing total disk clusters (line 1388). - Updated
GetDiskFreeSpaceWhook to useSPOOFED_DISK_SIZEandRECOVERY_PARTITION_SIZEconstants for spoofing total disk clusters (line 1411). - Removed trailing newline at the end of the file (line 1633).
- Updated
- hook_misc.c
- Updated
GetSystemInfohook to useSPOOFED_CPU_CORE_NUMconstant for spoofing the number of processors (line 767). - Updated
NtQuerySystemInformationhook to useSPOOFED_CPU_CORE_NUMconstant for spoofing the number of processors when queryingSystemBasicInformation(line 820).
- Updated
- hook_wmi.c
- Added a new source file
hook_wmi.ccontaining hook implementations for WMI functions:WMI_Get,WMI_ExecQuery,WMI_ExecQueryAsync,WMI_ExecMethod,WMI_ExecMethodAsync,WMI_GetObject,WMI_GetObjectAsync(lines 1-100). - These hooks primarily log the function calls and relevant arguments (like queries, object paths, method names).
- Added a new source file
- hooks.c
- Added a new macro
HOOK_WITHNAMEfor hooking functions by their mangled names (lines 41-42). - Added WMI hooks (
WMI_Get,WMI_ExecQuery,WMI_ExecMethod,WMI_ExecQueryAsync,WMI_ExecMethodAsync,WMI_GetObject,WMI_GetObjectAsync) to thefull_hooksarray, usingHOOK_WITHNAMEforWMI_GetandHOOK_NOTAILfor the others (lines 162-173).
- Added a new macro
- hooks.h
- Added function prototypes for the new WMI hooks (
WMI_Get,WMI_ExecQuery,WMI_ExecMethod,WMI_ExecQueryAsync,WMI_ExecMethodAsync,WMI_GetObject,WMI_GetObjectAsync) usingHOOKDEFandHOOKDEF_NOTAIL(lines 1264-1328).
- Added function prototypes for the new WMI hooks (
- log.c
- Added a new static helper function
log_variantto handle logging values stored inVARIANTstructures based on their type (lines 293-414). - Updated the
loqfunction to recognize and handle the new'n'format specifier for loggingVARIANTpointers usinglog_variant(lines 608-609, 793-795).
- Added a new static helper function
- log.h
- Added documentation for the new
'n'format specifier in the description of theloqfunction parameters (line 36).
- Added documentation for the new
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces WMI hooking capabilities, including Yara rules for detecting WMI functions and C hooks for intercepting them. It also centralizes several anti-analysis spoofing constants and updates existing hooks to use them. The addition of WMI monitoring is a significant enhancement.
There are a few critical and high-severity issues that need addressing, particularly concerning the behavior of the new WMI hooks and a bug in variant logging. Additionally, a medium-severity issue regarding integer literal suffixes for large constants should be reviewed.
Summary of Findings
- WMI Hook Behavior (Critical): Several new WMI hooks (
WMI_ExecQuery,WMI_ExecMethod, etc.) are defined usingHOOKDEF_NOTAILand registered such that theirOld_function pointers are not populated. They currently log parameters and returnS_OK(0) without calling the original WMI functions. This needs clarification: if monitoring and passthrough is intended, the hook registration and implementation must be changed to call the original functions. If stubbing/blocking is intended, this should be documented. - Incorrect Logging in
log_variant(High): The default case in the newlog_variantfunction inlog.cattempts to logvar->vt(a USHORT) as a string, which is incorrect and will lead to issues. It should log the formatted messagelog_msg. - Integer Literal Suffix for Large Constants (Medium): In
config.h,SPOOFED_GPU_RAMandSPOOFED_RAMuse thelsuffix for the value0x100000000. This value (4GB) can overflow a 32-bitlong. UsingULLorLLsuffix is recommended for correctness and consistency. - Missing Newline at End of File (Low - Not Commented): Files
config.h,hook_file.c, and the newhook_wmi.care missing a newline character at the end of the file. This is a minor style issue. - Potential
VARIANTLogging Enhancement (Low - Not Commented): Inhook_wmi.c, theWMI_Gethook has a commented-out line//LOQ_hresult("system", "un", "Name", wszName, "Value", pVal);. With the newlog_variantfunction and'n'specifier, this could potentially be enabled to log theVARIANTvaluepValif desired.
Merge Readiness
This pull request introduces valuable WMI hooking capabilities. However, there is a critical issue regarding the behavior of most new WMI hooks in hook_wmi.c: they currently do not call the original WMI functions. This needs to be addressed based on the intended design (monitoring passthrough vs. blocking/stubbing). Additionally, a high-severity bug in log_variant and a medium-severity issue with integer literal suffixes in config.h should be fixed.
Due to the critical nature of the WMI hook behavior, I recommend that these changes not be merged until this point is clarified and, if necessary, corrected. I am not authorized to approve pull requests, so further review and approval by other maintainers will be necessary after these issues are addressed.
| HOOKDEF_NOTAIL(WINAPI, WMI_ExecQuery, | ||
| PVOID _this, | ||
| const BSTR strQueryLanguage, | ||
| const BSTR strQuery, | ||
| LONG lFlags, | ||
| PVOID pCtx, | ||
| PVOID* ppEnum | ||
| ) { | ||
| HRESULT ret = 0; | ||
| LOQ_hresult("system", "u", "Query", strQuery); | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
The WMI hook functions WMI_ExecQuery, WMI_ExecQueryAsync, WMI_ExecMethod, WMI_ExecMethodAsync, WMI_GetObject, and WMI_GetObjectAsync are defined using HOOKDEF_NOTAIL. In hooks.c, these are initialized using the HOOK_NOTAIL macro, which sets the old_func pointer to NULL. This means these hook implementations currently cannot call their original Old_WMI_... counterparts and are instead returning 0 (S_OK) after logging.
Is the intention here to stub out these WMI functions, effectively making them no-ops that return success? Or is the goal to monitor these calls and then execute the original WMI functionality?
If monitoring and passthrough is intended (which is common for such hooks in an analysis tool), these functions should call their original counterparts. This would likely require:
- Using a different hook definition macro in
hooks.c(e.g.,HOOKorHOOK_NOTAIL_ALTif an alternate implementation is also needed) that correctly populates theOld_FunctionNamepointer. - Modifying these hook implementations to call
Old_WMI_FunctionName(...)and return its result.
For example, WMI_ExecQuery might look like:
HRESULT ret = Old_WMI_ExecQuery(_this, strQueryLanguage, strQuery, lFlags, pCtx, ppEnum);
LOQ_hresult("system", "u", "Query", strQuery);
return ret;(Assuming Old_WMI_ExecQuery is made available by changing the hook registration in hooks.c)
Could you clarify the intended behavior? If these calls are meant to be fully intercepted and not passed to the original WMI functions, this should be clearly documented as it's a significant behavior modification.
| snprintf(log_msg, 32, "Unhandled VARIANT Type: %hu", var->vt); | ||
| log_string((const char*)var->vt, -1); | ||
| break; |
There was a problem hiding this comment.
In the default case of the log_variant function, it appears there's an issue with how unhandled VARTYPEs are logged. The snprintf correctly prepares log_msg with the unhandled variant type, but then log_string is called with (const char*)var->vt.
var->vt is a USHORT representing the variant type, not a string. Casting it to const char* and attempting to log it as a string will lead to undefined behavior (likely a crash or garbage output).
Shouldn't this be logging the log_msg string instead, which contains the formatted error message?
snprintf(log_msg, 32, "Unhandled VARIANT Type: %hu", var->vt);
log_string(log_msg, -1); // Log the formatted message
break;| #define SPOOFED_GPU_RAM 0x100000000l // 4GB | ||
| #define SPOOFED_GPU_NAME L"NVIDIA GTX 1650" | ||
|
|
||
| #define SPOOFED_RAM 0x100000000l // 4GB | ||
| #define SPOOFED_RAM_RESERVED 0x410000 |
There was a problem hiding this comment.
The constants SPOOFED_GPU_RAM and SPOOFED_RAM are defined as 0x100000000l. The l suffix denotes a long int. The value 0x100000000 represents 4GB.
On systems where long is 32 bits (which is common, e.g., MSVC for 32-bit targets, and often on 64-bit Unix-like systems for long), this value (4,294,967,296) will overflow a signed 32-bit long (max ~2 billion) or wrap around an unsigned long (max 4,294,967,295).
To ensure these large constants are handled correctly and consistently across platforms, and to match the style of SPOOFED_DISK_SIZE (which uses ull), would it be better to use the ULL (unsigned long long) or LL (long long) suffix? For example: 0x100000000ULL.
| #define SPOOFED_GPU_RAM 0x100000000l // 4GB | |
| #define SPOOFED_GPU_NAME L"NVIDIA GTX 1650" | |
| #define SPOOFED_RAM 0x100000000l // 4GB | |
| #define SPOOFED_RAM_RESERVED 0x410000 | |
| #define SPOOFED_GPU_RAM 0x100000000ULL // 4GB | |
| #define SPOOFED_GPU_NAME L"NVIDIA GTX 1650" | |
| #define SPOOFED_RAM 0x100000000ULL // 4GB | |
| #define SPOOFED_RAM_RESERVED 0x410000 |
Thanks @KillerInstinct