search

elishacloud / dxwrapper

Fixes compatibility issues with older games running on Windows 10/11 by wrapping DirectX dlls. Also allows loading custom libraries with the file extension .asi into game processes.
zlib License
1.23k stars 90 forks source link

`EnumDisplaySettingsExA` hook function overwrites caller's stack #272

Closed BC46 closed 4 months ago

BC46 commented 4 months ago

I am working on a client-side plugin for a game that adds additional selectable resolutions to the options menu. This is done using several hook functions, in one of which I call EnumDisplaySettingsA to get the resolutions (https://github.com/BC46/FLSharp/blob/feature/better-resolutions/src/resolutions.cpp#L51; this is still a WIP, so the code is quite messy at the moment). Whenever I test this new feature without any DX8-hooks, it works fine. I also tried running it together with DxWrapper, but unfortunately this caused an access violation when one of my hooks with EnumDisplaySettingsA gets called. Under the hood EnumDisplaySettingsA is just a call to EnumDisplaySettingsExA, which is where DxWrapper's EnumDisplaySettingsExA hook function gets run.

It turns out that DxWrapper's EnumDisplaySettingsExA hook overwrites eight bytes of my hook function's stack, setting all the bytes to 0. As a result, my hook function's return address is set to 0x0 too, causing the program to jump to address 0x0 when the ret instruction is reached.

Using a hardware breakpoint, I traced the stack overwrite to a rep movsd instruction in the enumDisplaySettingsEx function, where 156 bytes are copied from the heap to the stack. Since the DEVMODEA struct is also 156 bytes, I am convinced that it has something to do with the typename DevMode allocation. Unfortunately after looking at the assembly and debug info for a little too long, I was unable to figure out how the problem could be potentially solved. Any hints would be greatly appreciated.

elishacloud commented 4 months ago

Using a hardware breakpoint, I traced the stack overwrite to a rep movsd instruction in the enumDisplaySettingsEx function, where 156 bytes are copied from the heap to the stack.

I wonder if the header should look more like this: enumDisplaySettingsEx

elishacloud commented 4 months ago

It could be that our hooks are colliding. I tried your code by itself in another application and I don't see any issue with dxwrapper. That part of the code was not written by me, but I reviewed it and did not see any issue.

Do you have a dbghelp.dll or dbgeng.dll file in the game folder? Try deleting both of those files.

Are you using the latest released build or the dev build of dxwrapper? The dev build here has a lot of bug fixes.

BC46 commented 4 months ago

I appreciate the quick reply!

It could be that our hooks are colliding.

This was also my initial suspicion. Though as long as the calling conventions, amount of parameters, stack allocations, etc are correct, everything should just be compatible.

Do you have a dbghelp.dll or dbgeng.dll file in the game folder? Try deleting both of those files.

I have neither. x64dbg says they are not loaded while the process is running: image

Are you using the latest released build or the dev build of dxwrapper?

At first I noticed the problem in an older dev build you provided in an issue opened by a different user (version 1.1.6972.22). Then I tried the latest release build, which did not cause a crash, but that's because EnumDisplaySettingsExA was not hooked yet in this build. After that I compiled the latest dev version locally, and while testing that build I could reproduce the crash, at which point I attempted to debug it.

BC46 commented 4 months ago

I just figured out what the problem is. I'll open a pull request in a bit.