Skip to content
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

Memory Map regression for remapped modules #2228

Closed
mappzor opened this issue Oct 15, 2019 · 2 comments

Comments

@mappzor
Copy link
Contributor

@mappzor mappzor commented Oct 15, 2019

I have problems with regression introduced by this commit: d50675c. I'm not sure which edge cases were fixed there but surely there are cases that got broken by this change. Everything works well when program is started under x64dbg but problem arises if you attach to a process which remapped certain modules in a specific way. Under those circumstances integer underflow will occurr and regions with negative sizes will appear in Memory Map.

I've prepared a small repro for this problem. Reproduction steps:

  1. Run TheMagic.exe (it spins infinitely after it does its job)
  2. Attach x64dbg
  3. Navigate to "Memory Map" and examine entries for TheMagic.dll
This is how it looks if you start TheMagic.exe under debugger (everything is OK)
Address           Size              Info                              Content                       Type   Protection
00007FFD0EC70000  0000000000110000                                                                  MAP    ER---        ER---
00007FFD0ED80000  0000000000090000                                                                  MAP    -R---        -R---
00007FFD0EE10000  0000000000010000                                                                  MAP    -RW--        -RW--
00007FFD0EE20000  00000000000B0000                                                                  MAP    -R---        -R---
00007FFD0EED0000  0000000000020000                                                                  MAP    -RW--        -RW--
00007FFD0EEF0000  0000000000023000                                                                  MAP    -R---        -R---

This is how it looks after mentioned commit (notice 2nd entry is malformed):
Address           Size              Info                              Content                       Type   Protection
00007FFD0EE40000  0000000000110000  themagic.dll                                                    MAP    ER---        ER---
00007FFD0F0C0000  FFFFFFFFFFD81000  themagic.dll                                                    MAP    -R---        -R---
00007FFD0EF50000  0000000000090000  themagic.dll                                                    MAP    -R---        -R---
00007FFD0EFE0000  0000000000010000  themagic.dll                                                    MAP    -RW--        -RW--
00007FFD0EFF0000  00000000000B0000  themagic.dll                                                    MAP    -R---        -R---
00007FFD0F0A0000  0000000000020000  themagic.dll                                                    MAP    -RW--        -RW--

This is how it looked before mentioned commit:
Address           Size              Info                              Content                       Type   Protection
00007FFD0F000000  0000000000110000  themagic.dll                                                    MAP    ER---        ER---
00007FFD0F110000  0000000000090000  themagic.dll                                                    MAP    -R---        -R---
00007FFD0F1A0000  0000000000010000  themagic.dll                                                    MAP    -RW--        -RW--
00007FFD0F1B0000  00000000000B0000  themagic.dll                                                    MAP    -R---        -R---
00007FFD0F260000  0000000000020000  themagic.dll                                                    MAP    -RW--        -RW--
00007FFD0F280000  0000000000023000  themagic.dll                                                    MAP    -R---        -R---

TheMagic.zip

@mrexodia

This comment has been minimized.

Copy link
Member

@mrexodia mrexodia commented Oct 16, 2019

Thanks, I will check it out! Unfortunately that code there is super disgusting spaghetti and needs some serious love, but up until that commit things were working ™️, so why change it right?

mappzor added a commit to mappzor/x64dbg that referenced this issue Nov 4, 2019
@mappzor

This comment has been minimized.

Copy link
Contributor Author

@mappzor mappzor commented Nov 4, 2019

I managed to fix that with one simple check. Assumptions of existing logic are now enforced.

However I found something interesting. I compared 2 scenarios:

  • program started under debugger
  • debugger attached to program

There are still discrepancies between those cases and it seems that Windows itself is at fault here. When unmapping view of section via NtUnmapViewOfSection, debug event is fired to notify debugger that module got unloaded. Usually that's fine because loader will also remove module from PEB. When unampping manually that obviously won't occurr, so detaching debugger and reattaching it will result in "dll loaded" event for dll that isn't loaded or got remapped as in this case.

Easiest fix for this would be to add extra validation logic in cbUnloadDll which would query modules from PEB via toolhelp API and check if module is actually gone. That's of course under assumption we want to trust PEB instead of debug events. Both solutions have their problems and are susceptible to manipulations. I'd like to know what's your take on this before investing more time into this.

@mrexodia mrexodia closed this in b0ba7d4 Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.