-
Notifications
You must be signed in to change notification settings - Fork 271
Improved GetProcAddress function emulation, DecoyModule structure and jitting process #257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
hi @williballenthin, |
williballenthin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see inline comments providing suggestions. feel free to push back on any of them (or implement if reasonable). let me know when we should merge. thank you!
speakeasy/windows/common.py
Outdated
|
|
||
| self.basepe = pefile.PE(data=husk, fast_load=True) | ||
|
|
||
| # set base properties |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| # set base properties |
speakeasy/windows/common.py
Outdated
|
|
||
| return exp_size | ||
|
|
||
| def align_file(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| def align_file(self): | |
| def pad_file(self): |
perhaps?
i think aligning would be changing the base address, not adding to the end
speakeasy/windows/common.py
Outdated
| self.add_section(name='.text', chars=0x60000020) | ||
| self.add_section(name='.edata', chars=0x40000040) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are there constants we could use for chars to make this more readable?
speakeasy/windows/common.py
Outdated
| return self.get_raw_pe() | ||
|
|
||
| def init_export_section(self, name, exports): | ||
| def init_export_section(self, name, exports, export_rvas): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does it make any senses to merge exports and export_rvas into a single object, rather than keeping them as parallel data structures?
| def generate_export_table(self, modname): | ||
| def generate_decoy_module(self, modname, base): | ||
| ''' | ||
| Generates a PE export table that can be parsed by malware |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you restore some of the documentation or perhaps make it more detailed? im not sure that "synthetic decoy module" has enough meaning to most people.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about:
Create a decoy module that emulates a DLL. The decoy module will contain basic
structures such as a .text section and an .edata section containing a list of exported functions.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great!
speakeasy/windows/winemu.py
Outdated
|
|
||
| if not path: | ||
| path = default_file_path | ||
| path = default_file_path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| path = default_file_path | |
| path = default_file_path |
|
Thank you for the great suggestions! I have updated my PR accordingly :) |
| def create_pattern(index): | ||
| if self.arch == _arch.ARCH_X86: | ||
| p = b'\x89\xff\x90\xB8' + index.to_bytes(4, 'little') + b'\xc3\x90\x90\x90\x90\x90\x90\x90' | ||
| else: | ||
| p = b'\x48\x89\xFF\x90\x48\xC7\xC0' + index.to_bytes(4, 'little') + b'\xc3\x90\x90\x90\x90' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would you add the disassembly of this in a comment, and possibly add a little human readable explanation as well?
Hi,
this PR was mainly created to address an issue in the GetProcAddress emulation.
The function always returns a value without checking whether the DLL actually exports the function. This causes problems under specific conditions. For example, the test binary in the PR tries to load FlsGetValue2, which is not exported by Kernel32 (at least on my system :P), resulting in an error.
I also added some additional improvements, in particular:
Thanks