Refactor DLL callbacks

[ASpoonPlaysGames]

cherry-picked and slightly modified from primedev, due to missing earlier commits from primedev

[GeckoEidechse]

@ASpoonPlaysGames just launching the game with this PR should be enough to verify that the PR works¹, right?

Footnotes

1. With the exemption of the Xinput dll of course. That also needs to be verified manually but cannot really do that on Linux I think ^^" ↩

[ASpoonPlaysGames]

As far as I know, yeah. Id go into a match just to make sure that everything is loaded and working though personally. Not sure why the xinput dll needs to be checked manually?

[GeckoEidechse]

Not sure why the xinput dll needs to be checked manually?

The last time we had a PR touch it, it accidentally broke the DLL override, meaning the Xinput DLL without ASLR was loaded again. Hence I'd prefer someone double checks that the right DLL is being loaded (don't remember what tool to use for that anymore) just so that we don't accidentally introduce a vulnerability into Northstar again :,)

[barnabwhy]

Not sure why the xinput dll needs to be checked manually?

it needs to be checked that XInput1_3.dll isn't being loaded and that XInput9_1_0.dll is instead just in case that behaviour was broken by this change

wow best me to it gecko 😔

[GeckoEidechse]

Related ¹

Last week, CyberpunkSaveEditor creator PixelRick disclosed that a buffer overflow vulnerability chained with a non-ASLR DLL (xinput1_3.dll) allows specially crafted save games or modifications to perform code execution on a PC.
https://www.bleepingcomputer.com/news/security/cyberpunk-2077-bug-fixed-that-let-malicious-mods-take-over-pcs/

(Not the best source but quickest I could find)

 

• https://learn.microsoft.com/en-us/windows/win32/xinput/xinput-versions

Footnotes

1. mostly putting it here so that I can reference it again in the future ↩

[GeckoEidechse]

Useful links

• https://learn.microsoft.com/en-us/sysinternals/downloads/listdlls
• Improve replacing xinput1_3 with xinput9_1 #583 (review)

[barnabwhy]

works for me, loads xinput 9.1.0 and not 1.3

[GeckoEidechse]

As per @F1F7Y's comment on Discord (https://discord.com/channels/920776187884732556/936105663942246410/1273620659167297610)

do not merge it
new code forgets to call g_pPluginManager->InformDllLoad
which in the old only some funcs call which is funky imo

requested change has been addressed

[barnabwhy]

worth noting this pr makes a change to the behaviour when xinput could not be loaded
previously an error message box would show and the game would exit
now there is just a error log and the game is allowed to run as normal

allowing the game to run without xinput will cause the game to silently accept no controller input on computers which for some reason don't have XInput9_1_0.dll in their System32 folder as they normally should
(tested by commenting this line)

imo this change is okay, but maybe in future there can be a more visible signifier to tell the player why the game isn't accepting controller input in order to prevent unnecessary confusion

this is wrong, the game will actually close if it can't find the dll

[barnabwhy]

everything now looks good to me
load success, game opens fine

load fail, error message shows

[F1F7Y]

Did not test. I don't think this should break anything, but still left some comments.

Ship it 💯

[GeckoEidechse]

Merging based on reviews.