Add Windows Kernel structs by HackingFrogWithSunglasses · Pull Request #1219 · qilingframework/qiling

HackingFrogWithSunglasses · 2022-08-31T16:17:37Z

As per #1217 this is the WIndows kernel structures PR

Checklist

Which kind of PR do you create?

This PR only contains minor fixes.
This PR contains major feature update.
[ * ] This PR introduces a new function/api for Qiling Framework.

Coding convention?

[ * ] The new code conforms to Qiling Framework naming convention.
[ * ] The imports are arranged properly.
[ * ] Essential comments are added.
The reference of the new code is pointed out.

Extra tests?

No extra tests are needed for this PR.
I have added enough tests for this PR.
[ * ] Tests will be added after some discussion and review.

Changelog?

This PR doesn't need to update Changelog.
Changelog will be updated after some proper review.
[ * ] Changelog has been updated in my PR.

Target branch?

[ * ] The target branch is dev branch.

One last thing

[ * ] I have read the contribution guide

elicn

Thanks for the contribution.
As mentioned on the previous PR you opened, the additional structures are not defined in a correct way. Please see BaseStruct documentation and how other Windows structures are defined to get the idea.

elicn · 2022-09-01T13:22:41Z

qiling/loader/pe.py


 from unicorn import UcError
-from unicorn.x86_const import UC_X86_REG_CR4, UC_X86_REG_CR8
+from unicorn.x86_const import UC_X86_REG_CR4, UC_X86_REG_CR8, UC_X86_REG_GS


UC_X86_REG_GS is imported but never used.

I think this should be used to write the kernel structures pointers to the GS register with their correct offset but I didn't manage to get it working so I wrote to the value manually.

elicn · 2022-09-01T13:24:32Z

qiling/loader/pe.py

+        kpcr_obj.Prcb = kprcb_addr          # Writes _KPRCB pointer to Prcb field
+
+        # Writes KPCR pointer into GS:[0x18]
+        self.ql.mem.write_ptr(0x6000018, kpcr_addr)


Please refrain from using magic numbers; there is a constant for GS that can be used.

This is not a magic number. That is the exact offset for the KPRCB pointer in Windows KM. I agree that the constant + offset should be used, though.

Great, that's what I meant.

elicn · 2022-09-01T13:27:01Z

qiling/loader/pe.py

+        self.ql.mem.map(kthread_addr, self.ql.mem.align_up(kthread_struct.sizeof()), info='[kthread]')
+
+        # Initialize an instance with key fields
+        kthread_obj = kthread_struct.volatile_ref(self.ql.mem, kthread_addr)


This structure is initialized with magic numbers whose origin is unclear.
Is there a place [a Qilign object] we can take the values from? Consistency is key.

I am not sure which numbers you're referring to are magic here.

Oh if you mean the 0x188 for GS, then same as above. Should be changed to constant + offset

I actually referred to the kthread_obj fields values: 11, 0x1000, 0x1500, 0x2000?

elicn · 2022-09-01T13:27:39Z

qiling/loader/pe.py

+        kthread_obj.InitialStack    = 0x1000
+        kthread_obj.StackBase       = 0x1500
+        kthread_obj.StackLimit      = 0x2000
+        kthread_obj.MiscFlags       |= 0x400


The value of MiscFlags is OR-ed with the existing one, but what is it...? That field was never initialized.

MiscFlags can be removed in this instance, I was using this for my own purpose and it won't be useful for Qiling generically.

elicn · 2022-09-01T13:29:15Z

qiling/loader/pe.py

+        # Writes KPRCB pointer into GS:[0x20]
+        self.ql.mem.write_ptr(0x6000020, kprcb_addr)
+
+        # @NOTE: Tests for writing structure pointers.


What this is about? Why is it so important it shouldn't be removed?
Also, Qiling debugging info should be logged as debug rather than warn.

You can remove these debug statements, I just had those there for my local branch and forgot to remove them when I submitted PR.

elicn · 2022-09-01T13:43:24Z

qiling/os/windows/structs.py

+        ('NestingLevel',                ctypes.c_char),
+        ('ClockOwner',                  ctypes.c_char),
+        ('PendingTickFlags',            ctypes.c_char),
+        ('PendingTick',                 ctypes.c_void_p),       # POS 0 : BIT 1


This is not how bit fields are defined.
To keep it simple, just keep the encapsulating field and do not define bit fields.

elicn · 2022-09-01T13:45:18Z

qiling/os/windows/structs.py

+    pointer_type = native_type
+
+    _fields_ = (
+        ('MxCsr',                       ctypes.c_ulong),


From the same reason mentioned above (avoiding structures definitions getting affected by the hosting arch), we should use only known size types, like c_uint32 and c_uint16 instead of long and short.

elicn · 2022-09-01T13:46:53Z

qiling/profiles/windows.ql

@@ -1,13 +1,17 @@
 [OS64]
-heap_address = 0x500000000
+heap_address = 0xfffff76000000000


Is there any specific reason for this change?

No specific reason relevant to Qiling. Was for my own testing purposes.

elicn · 2022-09-01T13:49:26Z

qiling/profiles/windows.ql

 entry_point = 0x140000000
-# KI_USER_SHARED_DATA = 0xfffff78000000000
-KI_USER_SHARED_DATA = 0x7ffe0000
+KI_USER_SHARED_DATA = 0xfffff78000000000


Is there any specific reason you removed the existing value and took the commented one?
Qiling doesn't have a concept of virtual and physical addresses, and this is the main reason why the value was changed in the first place.

No specific reason, again for my own testing.

qiling/profiles/windows.ql

HackingFrogWithSunglasses

I agree with the requested changes.

HackingFrogWithSunglasses · 2022-09-01T14:01:44Z

qiling/os/windows/structs.py

 # Cross Platform and Multi Architecture Advanced Binary Emulation Framework
 #

+from calendar import c


Agree, can be removed.

Apply fixes per discussion on PR.

HackingFrogWithSunglasses

👍

HackingFrogWithSunglasses · 2022-09-01T14:06:01Z

qiling/os/windows/structs.py


 from enum import IntEnum
 from functools import lru_cache
+from pickletools import uint1


Agree, can be removed.

HackingFrogWithSunglasses · 2022-09-01T14:06:49Z

qiling/os/windows/structs.py


    return PEB

+class NT_TIB(struct.BaseStruct):


The output is not needed but please be aware that with certain kernel structures you cannot just have a "link" to them because they are undocumented. In this case the output can be deleted though.

HackingFrogWithSunglasses · 2022-09-01T14:10:44Z

qiling/loader/pe.py


 from unicorn import UcError
-from unicorn.x86_const import UC_X86_REG_CR4, UC_X86_REG_CR8
+from unicorn.x86_const import UC_X86_REG_CR4, UC_X86_REG_CR8, UC_X86_REG_GS


I think this should be used to write the kernel structures pointers to the GS register with their correct offset but I didn't manage to get it working so I wrote to the value manually.

HackingFrogWithSunglasses · 2022-09-01T14:11:13Z

qiling/loader/pe.py

+        self.ql.mem.map(kthread_addr, self.ql.mem.align_up(kthread_struct.sizeof()), info='[kthread]')
+
+        # Initialize an instance with key fields
+        kthread_obj = kthread_struct.volatile_ref(self.ql.mem, kthread_addr)


I am not sure which numbers you're referring to are magic here.

HackingFrogWithSunglasses · 2022-09-01T14:41:40Z

qiling/loader/pe.py

+        self.ql.mem.map(kthread_addr, self.ql.mem.align_up(kthread_struct.sizeof()), info='[kthread]')
+
+        # Initialize an instance with key fields
+        kthread_obj = kthread_struct.volatile_ref(self.ql.mem, kthread_addr)


Oh if you mean the 0x188 for GS, then same as above. Should be changed to constant + offset

Apply fixes per discussion on PR.

elicn

I took the liberty to edit some of the files and apply the fixes myself.
Also added a few responses.

elicn · 2022-09-07T08:46:13Z

qiling/loader/pe.py

+        self.ql.mem.map(kthread_addr, self.ql.mem.align_up(kthread_struct.sizeof()), info='[kthread]')
+
+        # Initialize an instance with key fields
+        kthread_obj = kthread_struct.volatile_ref(self.ql.mem, kthread_addr)


I actually referred to the kthread_obj fields values: 11, 0x1000, 0x1500, 0x2000?

elicn · 2022-09-07T08:48:44Z

qiling/os/windows/structs.py


    return PEB

+class NT_TIB(struct.BaseStruct):


Yes, I am aware.. if there is an unformal documentation you can link to, that's perfectly fine.

HackingFrogWithSunglasses · 2022-09-07T08:59:32Z

That's absolutely fine as I don't have time myself to maintain this. Thanks!

Add Windows Kernel structs

ee00050

HackingFrogWithSunglasses mentioned this pull request Aug 31, 2022

Qdb x86-64 support & Windows kernel structures #1217

Closed

7 tasks

elicn requested changes Sep 1, 2022

View reviewed changes

HackingFrogWithSunglasses commented Sep 1, 2022

View reviewed changes

elicn added 2 commits September 7, 2022 11:39

Update pe.py

ccd89cc

Apply fixes per discussion on PR.

Update api.py

aa05300

Apply fixes per discussion on PR.

HackingFrogWithSunglasses commented Sep 7, 2022

View reviewed changes

elicn added 2 commits September 7, 2022 11:42

Update structs.py

e4c9b09

Apply fixes per discussion on PR.

Update windows.ql

b9f07c6

Apply fixes per discussion on PR.

elicn reviewed Sep 7, 2022

View reviewed changes

Comments

Conversation

HackingFrogWithSunglasses commented Aug 31, 2022

Checklist

Which kind of PR do you create?

Coding convention?

Extra tests?

Changelog?

Target branch?

One last thing

Uh oh!

elicn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

HackingFrogWithSunglasses left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HackingFrogWithSunglasses left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elicn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HackingFrogWithSunglasses commented Sep 7, 2022

Uh oh!