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

FusionFix fails to disable Zolika1351's Mods (3rd party ASI Plugins) #397

Closed
Sewer56 opened this issue Jan 7, 2024 · 10 comments
Closed

Comments

@Sewer56
Copy link

Sewer56 commented Jan 7, 2024

The following blacklist

std::vector<std::wstring> dlllist = {
// These A-Volute DLLs shipped with various audio driver packages improperly handle
// D3D9 exclusive fullscreen in a way that causes CreateDeviceEx() to deadlock.
// https://github.com/moonlight-stream/moonlight-qt/issues/102
L"NahimicOSD.dll", // ASUS Sonic Radar 3
L"SSAudioOSD.dll", // SteelSeries headsets
L"SS2OSD.dll", // ASUS Sonic Studio 2
L"Nahimic2OSD.dll",
L"NahimicMSIOSD.dll",
L"nhAsusPhoebusOSD.dll", // ASUS Phoebus
L"MirillisActionVulkanLayer.dll",
L"\x56\x52\x61\x64\x69\x6F\x2E\x61\x73\x69",
L"\x56\x57\x65\x61\x70\x6F\x6E\x2E\x61\x73\x69",
L"\x49\x56\x46\x69\x78\x65\x73\x49\x6D\x70\x72\x6F\x76\x65\x6D\x65\x6E\x74\x73\x2E\x61\x73\x69",
L"\x49\x56\x45\x78\x74\x72\x61\x4D\x69\x73\x73\x69\x6F\x6E\x73\x2E\x61\x73\x69",
L"\x47\x46\x57\x4C\x50\x72\x6F\x74\x65\x63\x74\x69\x6F\x6E\x44\x69\x73\x61\x62\x6C\x65\x72\x32\x30\x31\x39\x2E\x61\x73\x69",
L"\x43\x68\x61\x72\x53\x77\x69\x74\x63\x68\x41\x64\x64\x6F\x6E\x2E\x61\x73\x69",
L"\x49\x56\x53\x69\x64\x65\x41\x63\x74\x69\x76\x69\x74\x69\x65\x73\x4D\x50\x2E\x61\x73\x69",
L"\x49\x56\x53\x69\x64\x65\x41\x63\x74\x69\x76\x69\x74\x69\x65\x73\x2E\x61\x73\x69",
L"\x5A\x4D\x65\x6E\x75\x49\x56\x2E\x61\x73\x69",
L"\x49\x56\x54\x77\x65\x61\x6B\x65\x72\x2E\x61\x73\x69",
L"\x42\x6F\x64\x79\x67\x75\x61\x72\x64\x73\x4D\x6F\x64\x2E\x61\x73\x69",
L"\x53\x74\x65\x61\x6D\x41\x63\x68\x69\x65\x76\x65\x6D\x65\x6E\x74\x73\x2E\x61\x73\x69",
L"\x57\x61\x72\x64\x72\x6F\x62\x65\x4D\x6F\x64\x2E\x61\x73\x69",
L"\x49\x56\x4D\x65\x6E\x75\x41\x50\x49\x2E\x61\x73\x69",
L"\x5A\x6F\x6C\x69\x6B\x61\x50\x61\x74\x63\x68\x2E\x61\x73\x69",
L"\x44\x69\x76\x69\x6E\x67\x4D\x6F\x64\x2E\x61\x73\x69",
L"\x47\x54\x41\x49\x56\x45\x78\x74\x72\x61\x4F\x70\x74\x69\x6F\x6E\x73\x2E\x61\x73\x69",
};

comprises of other ASI plugins which are loaded by an ASI loader. However, assuming that UltimateAsiLoader is used as the code injection method used, this will fail to fully work.

This is because, UltimateAsiLoader queries the FileSystem via Windows API to obtain the list of files to be loaded, and loads them in the order returned via the API.

In other words, by the time FusionFix hooks LdrLoadDll, some of the mods in the blacklist may already be loaded.

The returned order of search items is not guaranteed as part of the Windows API. As far as I'm aware, it's entirely dependent on filesystem. For NTFS, this is alphabetical. So assuming GTAIV.EFLC.FusionFix.asi is the mod name, on common filesystems NTFS (I also believe Btrfs, ext4) it will fail to blacklist CharSwitchAddon.asi, GFWLProtectionDisabler2019.asi, BodyguardsMod.asi and DivingMod.asi from the given blacklist.

Doing a google search, it appears these mods were published by Zolika1351, hence the issue title.


!!! Note

I'm not a member of the GTA community, and I don't know much about FusionFix itself. I just saw this block of code while doing code search on GitHub noticed an obvious oversight in it. I've had to hook LdrLoadDll in the past due to 3rd party software either causing issues with either:

  • DirectX device resets
  • Using hooking libraries which overwrite the prologue of the original method when calling the original function.
    • Which is problematic for other API hooks. (e.g. For DirectX: Injecting ImGui, Screen Capture)

I was looking for an updated DLL list in case someone else was doing the same thing, and stumbled here.

It's also probably preferable that end users are notified of the presence of a blacklist; say for example, in a log file. Unexpected blacklist behaviour may lead to a degraded end user experience as things end users expect to work may suddenly disappear.

@ThirteenAG
Copy link
Owner

The function is not intended to block these. It was made for dlls that are loaded externally(and crash/hang dx for some reason). The list in question served it's purpose just few hours ago and will be deleted soon. It was, simply, a bait.

@ThirteenAG
Copy link
Owner

The remaining dlls I can probably add to the readme, or an ini file.

@Sewer56
Copy link
Author

Sewer56 commented Jan 7, 2024

The function is not intended to block these. It was made for dlls that are loaded externally(and crash/hang dx for some reason).

To provide some extra context for some of these.

In the case of the Nahimic drivers at least (preinstalled in some laptops), and RTSS (overlay that ships with MSI Afterburner), they mainly cause issue due to bad hooking libraries.

Their hooking libraries they use would override the function's entry point before calling original, and after calling original, they would override the code at function entry point with branch back to their hook. This meant they would undo any hooks that come after them. And if any hooks existed which had a branch/jmp with different length than theirs, it risked crashing.

The list in question served it's purpose just few hours ago and will be deleted soon. It was, simply, a bait.

Did this code not ship to end users?

Personally I get very paranoid with micro-optimization, so it feels like a waste of CPU cycles to include code which doesn't serve end user purpose. Originally I thought this was something to do with some sort of compat issues (e.g. incompatible hook functions with some common logic in another codebase). Well, I won't question it; but it's weird.

The remaining dlls I can probably add to the readme, or an ini file.

Yeah, that works.

Just something to make sure end users are aware; that's the most important part 😉


Feel free to close if needed; or with accompanying PR that addresses it, whatever works for you.

@ThirteenAG
Copy link
Owner

Did this code not ship to end users?

It does now, we kinda have a situation, zolika making up lies about this project and the team behind it, and this little list helped reveal that he just edits his posts to w/e suits him at the moment as we try to expose him. Very easily manipulated. So for example if I make this project private tomorrow, he will write I'm hiding something etc. So far he only managed to list things he did but instead claim it was done by us. Not to mention that he claimed he quit everything and won't be online to deal with the aftermath of his dealings, he edited his post 5 times already. A liar till the bitter end.

@Sewer56
Copy link
Author

Sewer56 commented Jan 7, 2024

Ah, a more personal issue. Well, it's probably best left at that for me then.

@RecklessGlue540
Copy link

DirectX device resets
Using hooking libraries which overwrite the prologue of the original method when calling the original function.
Which is problematic for other API hooks. (e.g. For DirectX: Injecting ImGui, Screen Capture)

I actually am still facing this problem when running the game with RivaTuner enabled... Now I likely know why : )
Thank you for this useful information!

@ThirteenAG
Copy link
Owner

Their hooking libraries they use would override the function's entry point before calling original, and after calling original, they would override the code at function entry point with branch back to their hook. This meant they would undo any hooks that come after them. And if any hooks existed which had a branch/jmp with different length than theirs, it risked crashing.

By the way, have you found a way to workaround the issue? For example, I wonder if switching minhook to safetyhook "fixes" the side effect of crashing/hanging? I wasn't able to reproduce the issue, so it's hard to test. RTSS works fine, I've asked someone to test.

@Sewer56
Copy link
Author

Sewer56 commented Jan 8, 2024

Re. RTSS

RTSS has I think over a year ago now added the option to use Detours as hooking library, which doesn't suffer from the issue. I don't daily drive Windows, so I don't know if it's now the default but it used to be opt in.

In any case, with RTSS, the actual hooks were fine (aside from the fact they were not thread safe), but the problem was that RTSS would save the function prologue at the time it hooks, so if you hooked a function after RTSS, RTSS would then undo your hook after calling original because it would restore the bytes that were present at the time it hooked the function originally.

As for the Nahimic drivers; I've never ran them, so I don't know the exact details. I found out by making a mod which logs loaded DLLs with the help of end users who historically had issues with overlays; then simply looking at what gets loaded.

You might be able to get away with hooking the prioprietary DLLs at load time to replace their hooking libraries with something better (through hooking them, ironically); though there is a chance the code to 'activate'/'deactivate' hook may get inlined, which would make it a bit more tricky to do as now you might end up having to write some custom assembly at multiple entry points.

@AlexUnwinder
Copy link

  • Using hooking libraries which overwrite the prologue of the original method when calling the original function.

    • Which is problematic for other API hooks. (e.g. For DirectX: Injecting ImGui, Screen Capture)

There is a concept of JMP chain unwinding, existing during at least a decade and supported by various hook architectures, which allows you to coexist with such hook implementations safely. The concept is simple: when you see a JMP installed by third party hook inside the function you're about to hook, never try to overwrite it with your own hook. Instead of doing so, just unwind JMP chain and inject your hook into module pointed by the very last JMP into the chain. This way you'll never overwrite JMP installed by hook of unknown architecture (which can be indeed patchable/unpatchable on the fly) and fundamentally exclude the risk of such conflicts.

@Sewer56
Copy link
Author

Sewer56 commented Jan 9, 2024

  • Using hooking libraries which overwrite the prologue of the original method when calling the original function.

    • Which is problematic for other API hooks. (e.g. For DirectX: Injecting ImGui, Screen Capture)

There is a concept of JMP chain unwinding, existing during at least a decade and supported by various hook architectures, which allows you to coexist with such hook implementations safely. The concept is simple: when you see a JMP installed by third party hook inside the function you're about to hook, never try to overwrite it with your own hook. Instead of doing so, just unwind JMP chain and inject your hook into module pointed by the very last JMP into the chain. This way you'll never overwrite JMP installed by hook of unknown architecture (which can be indeed patchable/unpatchable on the fly) and fundamentally exclude the risk of such conflicts.

That is a very valid approach; I've done something like this before, to make 6 byte absolute jumps interop with 5 byte relative jumps; I was patching return addresses of other hooks. In that case, it'd be the same approach, however you'd instead patch the return address into a branch to another hook.

This approach is fine for frameworks like UAL, where mod load order is unordered. However, in some other frameworks, including my own Reloaded-II, load order is strict. By contract, user can reorder mods and last to load should take precedence, so that's a no-go there. It depends on the use case, simply.

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

No branches or pull requests

4 participants