Feat: Migrate windows idt plug-in to volatility 3 #976

Ma1icious · 2023-06-29T08:30:53Z

I moved volatility2 windows idt plug-in to volatility 3 and is currently being tested somewhat, although the code is not very elegant.
Relevant Issue: #974

ikelos

Thanks very much for your submission!

This is a good first attempt, but there's a couple of minors points and a major shift in the way it operates which I'd strongly recommend. Where you've constructed your own objects (such as KPCR, etc) please consider instead defining a JSON ISF file, and defining class_types to override the standard struct class for any calculations/convenience methods that the objects should have.

You can find more information at:
https://volatility3.readthedocs.io/en/stable/complex-plugin.html#writing-using-intermediate-symbol-format-files
https://volatility3.readthedocs.io/en/stable/complex-plugin.html#writing-new-templates-and-objects

Or please ask on the slack channel #vol3-dev for help if you need it. 5:)

ikelos · 2023-07-01T16:37:46Z

volatility3/framework/plugins/windows/idt.py

+
+            # Get kpcr object
+            kpcr_offset = ntkrnlmp.get_type("_KPCR").relative_child_offset("PrcbData")
+            kpcr = ntkrnlmp.object(


It seems like you're trying to wrap the KPCR data into a custom object, rather than using a volatility object? Can I please strongly recommend that you consider defining a JSON file with the appropriate types for a _KPCR if the standard ones aren't suitable? You can even define an override class if there are specific methods or calculations you require (such as idt_entries or gdt_entries). This should reduce the complexity of the code and turn store more information, which should be data, as data rather than embedding it into code.

I don't recall whether it was the KPCR or the KD_DEBUGGER_DATA that started getting encrypted by Microsoft, but if that is the case for the KPCR, we may want a more generic/general means to handle it, so all the code can live in a central location in the core.

It was KD_DEBUGGER_DATA that gets encrypted now. AFAIK the _KPCR types are included in the IST files from ntoskrnl, so they're already available.

ikelos · 2023-07-01T16:41:33Z

volatility3/framework/plugins/windows/idt.py

+
+        for cpu_index in range(cpu_count):
+            # Calculate the address of KiProcessorBlock
+            KiProcessorBlock_addr = ntkrnlmp.get_symbol("KiProcessorBlock").address + cpu_index * 4


This seems to be manually iterating an array of pointers, but it would probably be better to just instantiate an object on top of the KiProcessorBlock offset, which knows that it contains an array of pointers to... whatever type of structure the pointers point to (I don't recall off the top of my head). It also doesn't look like the target attribute is set, so these are likely just pointers to... something. meaning when they're accessed, they're unlikely to work as expected?

I just looked at context.object_from_symbol and we current expect the symbol to contain type information to use that, but I might add a parameter, allowing plugins to construct an object from a symbol name and a type, just so we can reuse most of the existing machinery. For now you'd need to get the symbol and then instantiate it at the symbol's address (somewhat as you're doing here).

I've now created a branch to try to resolve this, #978 should allow an object_type parameter to be passed to module.object_from_symbol, with the same form as to module.object. Please give this a test and let me know on #978 if it functions as intended... 5:)

ikelos · 2023-07-01T16:43:32Z

volatility3/framework/plugins/windows/idt.py

+from volatility3.framework.configuration import requirements
+from volatility3.plugins.windows import modules
+
+GDT_DESCRIPTORS = dict(enumerate([


The can all be stored in data in the JSON ISF file, under the enums section. An example is, so the type and size must be defined, and then each of the possible names and associated values should be listed.

"ETW_COMPRESSION_RESUMPTION_MODE": { "base": "int", "constants": { "EtwCompressionModeNoDisable": 1, "EtwCompressionModeNoRestart": 2, "EtwCompressionModeRestart": 0 }, "size": 4 },

ikelos · 2023-07-01T16:45:08Z

volatility3/framework/plugins/windows/idt.py

+        kernel = self.context.modules[self.config["kernel"]]
+        layer_name = kernel.layer_name
+        symbol_table = kernel.symbol_table_name
+        kvo = self.context.layers[layer_name].config["kernel_virtual_offset"]


This should also be accessible as kernel.offset, I think.

ikelos · 2023-07-01T16:45:42Z

volatility3/framework/plugins/windows/idt.py

+        layer_name = kernel.layer_name
+        symbol_table = kernel.symbol_table_name
+        kvo = self.context.layers[layer_name].config["kernel_virtual_offset"]
+        ntkrnlmp = self.context.module(symbol_table, layer_name=layer_name, offset=kvo)


This should be the same as kernel?

ikelos · 2023-07-01T16:47:45Z

volatility3/framework/plugins/windows/idt.py

+                    sect_name = self.get_section_name(ntkrnlmp, layer_name, module, addr)
+                else:
+                    module_name = "UNKNOWN"
+                    sect_name = ''


Please use a class derived from BaseAbsentValue when indicating that data is unavailable or not relevant. This allows user interface designers to know more about how to display the information for the medium they're displaying the output.

ikelos · 2023-07-01T16:50:20Z

volatility3/framework/plugins/windows/idt.py

+        return TreeGrid(
+            [
+                ('CPU', int),
+                ('Index', str),


it isn't clear why Index is a str here, when it's actually already a number and then being converted into a string and have the 0x removed and set to uppercase? If it should be displayed as a number, please put is at an int, if it should be a hex value, please put hex. Again, it allows the user interface designers to know what kind of data is, how to align it, and will allow programs ingesting the data to use the actual value (for example if exported to CSV).

volatility3/framework/plugins/windows/idt.py

+            )
+            try:
+                yield idt_index, _KIDT(idt_struct)
+            except:


volatility3/framework/plugins/windows/idt.py

+
+            try:
+                yield gdt_index, _KGDT(gdt_struct)
+            except:


ikelos · 2023-07-01T18:08:32Z

volatility3/framework/plugins/windows/idt.py

+            for mod in mods:
+                if mod.DllBase + mod.SizeOfImage >= offset and mod.DllBase <= offset:
+                    return mod
+        except:


Please figure out exactly which type of exception you're intending to handle, and catch only that one. This allows for other errors that might not have been envisaged to bubble up through the code and potentially be handled more appropriately by something higher up. This goes for all try/except blocks in this code.

volatility3/framework/plugins/windows/idt.py

+            )
+            try:
+                yield idt_index, _KIDT(idt_struct)
+            except:


volatility3/framework/plugins/windows/idt.py

+
+            try:
+                yield gdt_index, _KGDT(gdt_struct)
+            except:


volatility3/framework/plugins/windows/idt.py

+            for mod in mods:
+                if mod.DllBase + mod.SizeOfImage >= offset and mod.DllBase <= offset:
+                    return mod
+        except:


Ma1icious · 2023-07-06T03:33:16Z

Thanks very much for your submission!

This is a good first attempt, but there's a couple of minors points and a major shift in the way it operates which I'd strongly recommend. Where you've constructed your own objects (such as KPCR, etc) please consider instead defining a JSON ISF file, and defining class_types to override the standard struct class for any calculations/convenience methods that the objects should have.

You can find more information at: https://volatility3.readthedocs.io/en/stable/complex-plugin.html#writing-using-intermediate-symbol-format-files https://volatility3.readthedocs.io/en/stable/complex-plugin.html#writing-new-templates-and-objects

Or please ask on the slack channel #vol3-dev for help if you need it. 5:)

Thanks for your reply! I have been busy with my work recently. I will make corrections according to the questions you raised when I am free.

iMHLv2 · 2023-09-27T12:35:19Z

volatility3/framework/plugins/windows/idt.py

+)
+
+
+class _KIDT:


@Ma1icious I'd recommend using "extensions" instead of this style of class representing _KIDT, _KGDT, and _KPCR. There are examples here:

https://github.com/volatilityfoundation/volatility3/tree/develop/volatility3/framework/symbols/windows/extensions

The advantage is the classes will then be accessible from all plugins, not just the IDT plugin. Also, it will eliminate all the lines like self.Offset = idt_struct.Offset as those will just be set naturally. The properties and methods can mostly stay the same, as they work on extensions as well.

iMHLv2 · 2023-09-27T17:27:44Z

volatility3/framework/plugins/windows/idt.py

+        self.symbol_table = symbol_table
+
+    def idt_entries(self):
+        base_idt = self.kpcr.IDT


I believe this may only work for x86. On x64, the _KPCR finds its IDT through a member named IdtBase instead. That may be OK if you only plan to support 32-bit systems, however the plugins' list of architectures include Intel64.

iMHLv2 · 2023-09-27T17:32:17Z

volatility3/framework/plugins/windows/idt.py

+        base_idt = self.kpcr.IDT
+        idt_index = 0
+        for idt_index in range(256):
+            idt_offset = base_idt + 8 * idt_index


A good shortcut here would be to use something similar to the code below (from the Windows handles.py file):

ptrs = ntkrnlmp.object( object_type="array", offset=table_addr, subtype=ntkrnlmp.get_type("pointer"), count=100, )

You could just create an array of 256 _KIDTENTRY entries and it would result in a list of objects. In that case, you wouldn't have to create 256 objects manually, and you could also remove the hard-coded 8 on line 102, because that would get calculated from the size of the _KIDTENTRY.

iMHLv2 · 2023-09-27T17:33:06Z

volatility3/framework/plugins/windows/idt.py

+
+        # Since the real GDT size is read from a register, we'll just assume
+        # that there are 128 entries (which is normal for most OS)
+        for gdt_index in range(128):


Same comment as above regarding the use of array. The code here isn't wrong, it could just be simplified a bit and remove some hard coded values.

iMHLv2 · 2023-09-27T17:34:51Z

volatility3/framework/plugins/windows/idt.py

+
+            # Get kpcr object
+            kpcr_offset = ntkrnlmp.get_type("_KPCR").relative_child_offset("PrcbData")
+            kpcr = ntkrnlmp.object(


It was KD_DEBUGGER_DATA that gets encrypted now. AFAIK the _KPCR types are included in the IST files from ntoskrnl, so they're already available.

ikelos · 2023-09-28T16:19:06Z

@Ma1icious there are a number of outstanding changes and comments made from our reviews. Could you please address them so that we can get the code merged?

ikelos · 2024-02-01T11:05:36Z

No response from the author so converting this to a draft.

ikelos · 2024-10-06T17:02:01Z

volatility3/framework/plugins/windows/idt.py

+            )
+
+            # Get kpcr object
+            kpcr_offset = ntkrnlmp.get_type("_KPCR").relative_child_offset("PrcbData")


#1280 suggest that PrcbData may not always be available in all versions, this should probably either test for it, or be wrapped in a protective try/except to avoid errors making their way back to the user.

Ma1icious added 2 commits June 29, 2023 16:28

Feat: Migrate windows idt plug-in to volatility 3

d6d94d6

Fix: remove unnecessary calls in windows.idt.IDT

85c918e

ikelos reviewed Jul 1, 2023

View reviewed changes

github-advanced-security bot found potential problems Jul 1, 2023

View reviewed changes

ikelos assigned iMHLv2 Jul 5, 2023

Ma1icious and others added 2 commits July 7, 2023 10:09

Merge branch 'volatilityfoundation:develop' into develop

d530d2f

Fix[Win_IDT]: Use black to format the code

1b64953

digitalisx mentioned this pull request Aug 13, 2023

Feature: implement Timers plugin. #695

Closed

5 tasks

iMHLv2 requested changes Sep 27, 2023

View reviewed changes

ikelos marked this pull request as draft February 1, 2024 11:05

ikelos added the needs-more-info label Mar 13, 2024

ikelos reviewed Oct 6, 2024

View reviewed changes

ikelos mentioned this pull request Oct 6, 2024

Regrading volatility plugin IDT #1280

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: Migrate windows idt plug-in to volatility 3 #976

Feat: Migrate windows idt plug-in to volatility 3 #976

Ma1icious commented Jun 29, 2023

ikelos left a comment

ikelos Jul 1, 2023

iMHLv2 Sep 27, 2023

ikelos Jul 1, 2023

ikelos Jul 1, 2023

ikelos Jul 1, 2023

ikelos Jul 1, 2023

ikelos Jul 1, 2023

ikelos Jul 1, 2023

ikelos Jul 1, 2023

ikelos Jul 1, 2023

ikelos Jul 1, 2023

Ma1icious commented Jul 6, 2023

iMHLv2 Sep 27, 2023

iMHLv2 Sep 27, 2023

iMHLv2 Sep 27, 2023

iMHLv2 Sep 27, 2023

iMHLv2 Sep 27, 2023

ikelos commented Sep 28, 2023

ikelos commented Feb 1, 2024

ikelos Oct 6, 2024

Feat: Migrate windows idt plug-in to volatility 3 #976

Are you sure you want to change the base?

Feat: Migrate windows idt plug-in to volatility 3 #976

Conversation

Ma1icious commented Jun 29, 2023

ikelos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ma1icious commented Jul 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ikelos commented Sep 28, 2023

ikelos commented Feb 1, 2024

Choose a reason for hiding this comment