Skip to content

Fix bug: DLL address still use ImageBase address in spite of changing lib_base.#801

Closed
nacayoshi00 wants to merge 17 commits into
qilingframework:old_devfrom
nacayoshi00:add_dll_reload_image
Closed

Fix bug: DLL address still use ImageBase address in spite of changing lib_base.#801
nacayoshi00 wants to merge 17 commits into
qilingframework:old_devfrom
nacayoshi00:add_dll_reload_image

Conversation

@nacayoshi00
Copy link
Copy Markdown

… lib_base

Checklist

Which kind of PR do you create?

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

Coding convention?

  • [ x ] 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?

  • [ x ] 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.
  • [ x ] Changelog will be updated after some proper review.
  • Changelog has been updated in my PR.

Target branch?

  • [ x ] The target branch is dev branch.

One last thing


@xwings
Copy link
Copy Markdown
Member

xwings commented May 25, 2021

There are some test with fixed based DLL address. Will you be able to update the test ?

@nacayoshi00
Copy link
Copy Markdown
Author

I understood. I will add the test to test_pe.py. Please be patient.

@nacayoshi00
Copy link
Copy Markdown
Author

nacayoshi00 commented Jun 1, 2021

I updated test_pe.py to check the fix of lib address relocation.
And I also update 2 fixes as below:

  • Implement PE loader to load all of the DLLs recursively when PE loaded.
  • Implement PE loader to be able to load API DLL (like api-ms-xxxx.dll).

I added a sample PE file for the test. So Should I send a pull request to https://github.com/qilingframework/rootfs/tree/master?

@xwings
Copy link
Copy Markdown
Member

xwings commented Jun 2, 2021

I updated test_pe.py to check the fix of lib address relocation.
And I also update 2 fixes as below:

  • Implement PE loader to load all of the DLLs recursively when PE loaded.
  • Implement PE loader to be able to load API DLL (like api-ms-xxxx.dll).

I added a sample PE file for the test. So Should I send a pull request to https://github.com/qilingframework/rootfs/tree/master?

Yes. If there is a new exe, you need to update rootfs

@nacayoshi00
Copy link
Copy Markdown
Author

OK, I sent the pull request of rootfs. And fix some code to pass the test

Comment thread examples/scripts/dllscollector.bat Outdated
if exist %WINDIR%\SysWOW64\downlevel\api-ms-win-core-fibers-l1-1-1.dll xcopy /f /y %WINDIR%\SysWOW64\downlevel\api-ms-win-core-fibers-l1-1-1.dll "examples\rootfs\x86_windows\Windows\System32\"
if exist %WINDIR%\SysWOW64\downlevel\api-ms-win-core-localization-l1-2-1.dll xcopy /f /y %WINDIR%\SysWOW64\downlevel\api-ms-win-core-localization-l1-2-1.dll "examples\rootfs\x86_windows\Windows\System32\"
if exist %WINDIR%\SysWOW64\downlevel\api-ms-win-core-sysinfo-l1-2-1.dll xcopy /f /y %WINDIR%\SysWOW64\downlevel\api-ms-win-core-sysinfo-l1-2-1.dll "examples\rootfs\x86_windows\Windows\System32\"
xcopy /f /y %WINDIR%\SysWOW64\downlevel\api-*.dll "examples\rootfs\x86_windows\Windows\System32\"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to limit and copy only needed DLL. It seems the test took 4 hours due to additional dlls

@nacayoshi00
Copy link
Copy Markdown
Author

OK, I updated examples/scripts/dllscollector.bat to copy only needed DLL.

@xwings
Copy link
Copy Markdown
Member

xwings commented Jun 5, 2021

Redoing the test and see how long will it take to do the test.

Comment thread qiling/loader/pe.py Outdated

if self.ql.archtype == QL_ARCH.X86:
address = self.ql.pack32(addr)
# load all DLL files linked from the target PE file recursively and make IAT of all dlls.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here onwards

The test before this take around 15-20 min. With the new implementation. It takes around 4 hours to complete the test.

We need to find a better way, This is way too slow.

One method is to cache like how we cache the dll address. maybe we can do the same

Comment thread qiling/loader/pe.py Outdated
self.ql.log.warning(f' - {warning}')

# [Room for Improvement] too much time when kernelbase.dll is loaded.
self.ql.log.debug('relocate {}, {:x}'.format(dll_name, self.dll_last_address))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to standardize all the quote. Double quote. " and not '

@nacayoshi00
Copy link
Copy Markdown
Author

I see, but the result was unexpected for me because the test took only 1638s on my laptop PC.
I think the reason why the test is so long is below:

  • All DLLs are loaded at first even if some DLLs may not be used.
  • DLL reload_image() may take quite long

Since I think these fixes are needed because emulation fails when DLL's address is not resolved, I have another solution to solve the problem.

  1. Make resolve_dll_address function that resolves address when emulator encounters unmapped address.
  2. Register the function to hook_address.

If u r OK, I'll try to update.

@xwings
Copy link
Copy Markdown
Member

xwings commented Jun 7, 2021

1638s is one test or all test ?

The test is being done from github CI, so i guess we need to make that as benchmark.

I guess your idea is fine. We need to fix CI result. Else gonna be bad if we need to spend 4 hours for each test. Since currently only 10min for all test.

@nacayoshi00
Copy link
Copy Markdown
Author

It took 1638s to finish all the tests.
OK, I'll try to fix it. Please be patient.

@xwings
Copy link
Copy Markdown
Member

xwings commented Jun 9, 2021

Sure. Will be waiting :)

@nacayoshi00
Copy link
Copy Markdown
Author

I'm sorry for late the response. I update these files to load dll dynamically.
But the problem still remained because the reason why take quite so long is to relocate some dlls such as shell32.dll (see below log.). So I add libcache=True in all_tests of test_pe.py.

>>> timeit.timeit("import pefile;pefile.PE('qiling/examples/rootfs/x86_windows/Windows/System32/shell32.dll').relocate_image(0x7fff00000)", number=1) 205.60229980000076

I checked the time dramatically reduced in my PC when libcache is enabled.

  • previous ver:1638s
  • new version(libcache off): 1000s
  • new version(libcache on): 80s)

@nacayoshi00
Copy link
Copy Markdown
Author

I resolved the conflict and re-uploaded

@nacayoshi00
Copy link
Copy Markdown
Author

Could you mind if I ask you to run the workflow? Or should I do something to proceed with the workflow?

@xwings
Copy link
Copy Markdown
Member

xwings commented Aug 12, 2021

There is still a conlifct with wscok32

@nacayoshi00
Copy link
Copy Markdown
Author

Sorry too late. I removed the conflict. So could you please run the workflow?

@wtdcode wtdcode deleted the branch qilingframework:old_dev October 12, 2021 08:49
@wtdcode wtdcode closed this Oct 12, 2021
@wtdcode wtdcode reopened this Oct 14, 2021
@elicn
Copy link
Copy Markdown
Member

elicn commented Apr 24, 2022

Hi @nacayoshi00,
Based on the recent changes made to dev branch, could you please test to see whether this PR is still relevant?

@xwings
Copy link
Copy Markdown
Member

xwings commented Oct 6, 2022

Since author did not repnse since Aug 2021. PR will be close for now.

@xwings xwings closed this Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants