Windows: Add support for missing callback types #1110
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks, there's a lot to go through here but it generally looks pretty good. It's quite a significant chunk of functionality though so I'd quite like to defer to @iMHLv2 's opinion to ensure it does what it's supposed and there's no gotchas that I've missed...
dgmcdona
force-pushed
the
dgmcdona/windows-callbacks
branch
3 times, most recently
from
186c814
to
84f8997
Compare
Updates the windows callbacks symbol files to include structures that were missing from the original volatility plugin.
Fixes a bad format string inside of a raised exception
Updates the Windows callbacks plugin to support several types of callbacks that were present in the original volatility framework, but were missing in volatility3. Adds an extension for `_SHUTDOWN_PACKET` structures for determining validity of structure.
This plugin class was missing a `_version` attribute, so I added one and set it to (1, 0, 0).
dgmcdona
force-pushed
the
dgmcdona/windows-callbacks
branch
from
84f8997
to
953b433
Compare
| from volatility3.framework.configuration import requirements | ||
| from volatility3.framework.interfaces.objects import ObjectInterface |
There was a problem hiding this comment.
Please try not to import classes directly, but only ever modules. Importing ObjectInterface like this may allow someone to one day write from callbacks import ObjectInterface and it then won't be clear that it actually belongs to another module.
| The list containing the built constraints. | ||
| """ | ||
|
|
||
| PNP9_SIZE = 0x30 |
There was a problem hiding this comment.
I'm generally a fan of having fixed values pulled out and tagged with a comment or put in a variable, but it doesn't feel like these will get used anywhere else, so I'd be tempted to just embed them in the code below? The code below already kinda hard codes the binary tag values so I'm not sure it's much different?
| context, layer_name, nt_symbol_table, constraints | ||
| ): | ||
| try: | ||
| if hasattr(mem_object, "is_valid") and not mem_object.is_valid(): |
There was a problem hiding this comment.
What objects don't have an is_valid method? Feels like this could be changed to just check pointers?
|
|
||
| return ( | ||
| callback_type, | ||
| cast(int, callback_address), |
There was a problem hiding this comment.
Yeah, probably a little overboard with the casting... 5;) I mean, it's good and I approve of well typed systems, but... a lot of these things should already be the type they're cast to, and hopefully mypy will one day be able to figure that out, but I'm not sure littering our code with hints will help all that much?
|
@iMHLv2 could you give this a scan over to check there's nothing subtle I've missed please? |
This PR updates the windows.callbacks.Callbacks plugin to support callback types that were present in the original volatility framework but have not yet been added to volatility3. These callback types include:
IoRegisterShutdownNotificationIoRegisterFsRegistrationChangeGenericKernelCallbackEventCategoryHardwareProfileChangeEventCategoryDeviceInterfaceChangeEventCategoryTargetDeviceChangeDbgSetDebugPrintCallbackThis required updates to the callbacks JSON symbol files, the creation of a
_SHUTDOWN_PACKETextension, and updates to the plugin itself. Because it introduces three new requirements (handles, driverirp, and poolscanner), I have incremented the major version number for the callbacks plugin. No other plugins depend on the callbacks plugin at this time, so it was not necessary to increase version numbers in other plugins.