From 8ad592a490f23b27e6f79e1cbff7b67a7c2f8aba Mon Sep 17 00:00:00 2001 From: David McDonald Date: Mon, 30 Sep 2024 11:00:58 -0500 Subject: [PATCH 1/2] Windows: Fixes bad callback validity check This fixes a bug in the callbacks plugin which causes it to miss `IoRegisterShutdownNotification` callbacks on x86b samples. The `header.NameInfo.Name` field was being incorrectly treated as the device type. This fixes the issue by updating the `is_valid` method on the `_SHUTDOWN_PACKET` extension type to take a `type_map` parameter, and updates the method to correctly validate the object type. --- .../framework/plugins/windows/callbacks.py | 8 ++++++-- .../symbols/windows/extensions/callbacks.py | 19 +++++++++++++++---- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/volatility3/framework/plugins/windows/callbacks.py b/volatility3/framework/plugins/windows/callbacks.py index d5eeda1ea..562846def 100644 --- a/volatility3/framework/plugins/windows/callbacks.py +++ b/volatility3/framework/plugins/windows/callbacks.py @@ -248,8 +248,12 @@ class Callbacks(interfaces.plugins.PluginInterface): context, layer_name, nt_symbol_table, constraints ): try: - if hasattr(mem_object, "is_valid") and not mem_object.is_valid(): - continue + if isinstance(mem_object, callbacks._SHUTDOWN_PACKET): + if not mem_object.is_parseable(type_map): + continue + elif hasattr(mem_object, "is_valid"): + if not mem_object.is_valid(): + continue yield cls._process_scanned_callback(mem_object, type_map) except exceptions.InvalidAddressException: diff --git a/volatility3/framework/symbols/windows/extensions/callbacks.py b/volatility3/framework/symbols/windows/extensions/callbacks.py index f894644db..62e72803e 100644 --- a/volatility3/framework/symbols/windows/extensions/callbacks.py +++ b/volatility3/framework/symbols/windows/extensions/callbacks.py @@ -1,4 +1,5 @@ import logging +from typing import Dict from volatility3.framework import exceptions, objects from volatility3.framework.symbols.windows.extensions import pool @@ -14,7 +15,7 @@ class _SHUTDOWN_PACKET(objects.StructType, pool.ExecutiveObject): It exposes a function which sanity-checks structure members. """ - def is_valid(self) -> bool: + def is_parseable(self, type_map: Dict[int, str]) -> bool: """ Perform some checks. """ @@ -24,6 +25,9 @@ class _SHUTDOWN_PACKET(objects.StructType, pool.ExecutiveObject): and self.Entry.Blink.is_readable() and self.DeviceObject.is_readable() ): + vollog.debug( + f"Callback obj 0x{self.vol.offset:x} invalid due to unreadable structure members" + ) return False device = self.DeviceObject @@ -41,10 +45,17 @@ class _SHUTDOWN_PACKET(objects.StructType, pool.ExecutiveObject): try: header = device.get_object_header() - valid = header.NameInfo.Name == "Device" - return valid + object_type = header.get_object_type(type_map) + is_valid = object_type == "Device" + if not is_valid: + vollog.debug( + f"Callback obj 0x{self.vol.offset:x} invalid due to invalid device type: wanted 'Device', found '{object_type}'" + ) + return is_valid except ValueError: - vollog.debug(f"Could not get NameInfo for object at 0x{self.vol.offset:x}") + vollog.debug( + f"Could not get object type for object at 0x{self.vol.offset:x}" + ) return False From db55f23070b16d8c9df40e1ddfda5df8a6ad3282 Mon Sep 17 00:00:00 2001 From: David McDonald Date: Mon, 30 Sep 2024 16:55:27 -0500 Subject: [PATCH 2/2] Windows: Callbacks - fix breaking API change Moves as much of the `is_parseable` check as possible back into an `is_valid` method to avoid breaking API changes. --- .../symbols/windows/extensions/callbacks.py | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/volatility3/framework/symbols/windows/extensions/callbacks.py b/volatility3/framework/symbols/windows/extensions/callbacks.py index 62e72803e..f54db39f2 100644 --- a/volatility3/framework/symbols/windows/extensions/callbacks.py +++ b/volatility3/framework/symbols/windows/extensions/callbacks.py @@ -15,7 +15,7 @@ class _SHUTDOWN_PACKET(objects.StructType, pool.ExecutiveObject): It exposes a function which sanity-checks structure members. """ - def is_parseable(self, type_map: Dict[int, str]) -> bool: + def is_valid(self) -> bool: """ Perform some checks. """ @@ -30,6 +30,25 @@ class _SHUTDOWN_PACKET(objects.StructType, pool.ExecutiveObject): ) return False + except exceptions.InvalidAddressException: + vollog.debug( + f"callback obj 0x{self.vol.offset:x} invalid due to invalid address access" + ) + return False + + return True + + def is_parseable(self, type_map: Dict[int, str]) -> bool: + """ + Determines whether or not this `_SHUTDOWN_PACKET` callback can be reliably parsed. + Requires a `type_map` that maps NT executive object type indices to string representations. + This type map can be acquired via the `handles.Handles.get_type_map` classmethod. + """ + if not self.is_valid(): + return False + + try: + device = self.DeviceObject if not device or not (device.DriverObject.DriverStart % 0x1000 == 0): vollog.debug( @@ -37,13 +56,6 @@ class _SHUTDOWN_PACKET(objects.StructType, pool.ExecutiveObject): ) return False - except exceptions.InvalidAddressException: - vollog.debug( - f"callback obj 0x{self.vol.offset:x} invalid due to invalid address access" - ) - return False - - try: header = device.get_object_header() object_type = header.get_object_type(type_map) is_valid = object_type == "Device" @@ -52,6 +64,11 @@ class _SHUTDOWN_PACKET(objects.StructType, pool.ExecutiveObject): f"Callback obj 0x{self.vol.offset:x} invalid due to invalid device type: wanted 'Device', found '{object_type}'" ) return is_valid + except exceptions.InvalidAddressException: + vollog.debug( + f"callback obj 0x{self.vol.offset:x} invalid due to invalid address access" + ) + return False except ValueError: vollog.debug( f"Could not get object type for object at 0x{self.vol.offset:x}"