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.15k stars 82 forks source link

Fix rare buffer overflow in `EnumDisplaySettingsEx` hook #273

Closed BC46 closed 1 month ago

BC46 commented 1 month ago

Please see https://github.com/elishacloud/dxwrapper/issues/272. Looking a bit more at the technical details, I discovered the following: Since I am using a rather old C++ compiler to build my plugin, sizeof(DEVMODEA) actually evaluates to 148, not 156. As a result, my plugin allocates 148 bytes on the stack for the device mode struct. When dxwrapper's EnumDisplaySettingsExA hook modifies the value that lpDevMode points to, it assumes 156 bytes, causing it to overwrite eight stack bytes too many.

This pull request makes it so that the device mode size in the enumDisplaySettingsEx function is no longer assumed, but instead uses the dmSize value that the caller passes. These changes fix the issue I've mentioned, and in my opinion it's the simplest way to go about it. However, with this approach the devModes vector will still store those eight additional bytes per element when they're not necessary. Though given the circumstances, fixing this the "ideal" way may be a bit difficult. I am open to any other suggestions.

Resolves https://github.com/elishacloud/dxwrapper/issues/272.

elishacloud commented 1 month ago

Thanks for catching this! I think the solution is good. I guess dmPanningWidth and dmPanningHeight are not always included in all structures. I think leaving them as 0 is probably not going to hurt anything.