ME3Tweaks / ALOTInstaller

Installer for A Lot of Textures for Mass Effect, Mass Effect 2, and Mass Effect 3
GNU General Public License v3.0
51 stars 10 forks source link

V4 Checklist #23

Closed Mgamerz closed 3 years ago

Mgamerz commented 3 years ago

Things that need to be done for V4

mirh commented 3 years ago

What about it? (I'm trying to make ends meet with the beginning of the school year as of lately)

Mgamerz commented 3 years ago

I just implemented Legacy PhysX installation code, as you requested like 50 years ago. When you have some time, was wondering if you could check it out. I found some posts of yours on ancient steam forums that this should satisfy when the game doesn't run as admin. V4 does not modify the AGEIA key anymore.

mirh commented 3 years ago

Everything that comes to my mind:

Mgamerz commented 3 years ago

Cause if nvidia decides to change the link, or no longer host it, it doesn't work anymore. I have no problem storing binaries in my source control that are used by the application, there are multiple files that are downloaded by every client that are stored in the repo.

I noticed that once I installed legacy PhysX, my newer physx games stopped working (ME2,ME3).

I'm not sure editing the registry key is a good idea, as it may break other games, which kind of makes the whole point moot. I was going to look it up in ageia key but was pretty lazy, so I just went to the local file detection route. Probably should update that before release.

mirh commented 3 years ago

The link is the exact same one since 7 years tbh.

I'm not sure editing the registry key is a good idea, as it may break other games

As I wrote, the presence of the key is what breaks other games. The real proper fix would be to remove set value access to the administrator account in fact, but then that could actually compromise main physx updates (for as much as it's already more than a year that PSS hasn't seen any activity)

I noticed that once I installed legacy PhysX, my newer physx games stopped working (ME2,ME3).

Wtf really? Legacy shouldn't interfere in any way with 2.8.

Mgamerz commented 3 years ago

As for PhysX issue on newer games, I'll do some more testing to see what's going on. I'm wondering if I had a newer PhysX that I uninstalled beforehand, to ensure there was no PhysX installed (as I could not remember if I had legacy PhysX installed). However, reinstalling them me3 one still left ME2's version broken, I'm going to guess it doesn't install older versions with the newer versions. I coded this way too late at night.

When you say presence of key, do you mean AGEIA or the data value for enableLocalPhysXCore?

mirh commented 3 years ago

I'm wondering if I had a newer PhysX that I uninstalled beforehand, to ensure there was no PhysX installed

That may indeed be it. For some reason ME2 will always want to load the system physx version no matter what.

However, reinstalling them me3 one still left ME2's version broken

That's very strange (assuming I got the default redists right). Besides the legacy branches, all installers should contain all the versions.

When you say presence of key, do you mean AGEIA or the data value for enableLocalPhysXCore?

Sorry for confusion, I meant the latter.

Mgamerz commented 3 years ago

In the latest V4 code there's now a better check for Legacy PhysX, per your guidance. Right now it doesn't delete enableLocalPhysXCore (I haven't had time to do it), but it won't install PhysX either if the ageia key itself is writable (which means it was previously modified by ALOT installer). This will fix it for new users, but i'm going to leave existing users as they are cause I don't want to break things further. Hopefully I find time to handle enableLocalPhysXCore in the next few days.

mirh commented 3 years ago

Thank you for the detailed prompt explanation!

I don't think you should hardcode the 64-bit registry view though. AFAIK proper practice when dealing with these idiosyncrasies is using RegistryKey.OpenBaseKey(Microsoft.Win32.RegistryHive.LocalMachine, RegistryView.Registry32)

And you still are also hardcoding the physx path (check the PhysXCore Path key) And behold, I just figured out a permission whose denying can fool ME1 from writing, whilst still allowing the physx installers to do whatever they want! ... I think I'd just better post the flow chart I have in my mind rather than just talk:

if (not AGEIATechnologies key exists || not 2.7.2 value exists || not legacyPhysXEngineFolder){
  download/install legacy physx
}
if (not AGEIATechnologies permissions as expected){
   set permissions for administrator group to "full control minus create subkey"
}

All this testing left me a bit tiddly, I hope I didn't forget anything.

Mgamerz commented 3 years ago

Hopefully this fixes it. I've been receiving a worrying number of reports that ME1 is no longer launching from origin. But there are no crash logs, so it's not even running. It runs when doing origin as admin though, which means origin may have changed their launch rules... Happens on both old and new installers so it's not like it is the new installer.

Mgamerz commented 3 years ago

Also - I used the 64 bit view cause we don't support 32bit. The app won't even run or work on 32 bit as the compression lib from MEM is 64bit only and the installers are also only built for x64.

On Sun, Nov 1, 2020, 7:16 AM mirh notifications@github.com wrote:

Thank you for the detailed prompt explanation!

I don't think you should hardcode the 64-bit registry view though. AFAIK proper practice when dealing with these idiosyncrasies is using RegistryKey.OpenBaseKey(Microsoft.Win32.RegistryHive.LocalMachine, RegistryView.Registry32)

And you still are also hardcoding the physx path (check the PhysXCore Path key) And behold, I just figured out a permission whose denying can fool ME1 from writing, whilst still allowing the physx installers to do whatever they want! ... I think I'd just better post the flow chart I have in my mind rather than just talk:

if (not AGEIATechnologies key exists || not 2.7.2 value exists || not legacyPhysXEngineFolder){ download/install legacy physx } if (not AGEIATechnologies permissions as expected){ set permissions for administrator group to "full control minus create subkey" }

All this testing left me a bit tiddly, I hope I didn't forget anything.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ME3Tweaks/ALOTInstaller/issues/23#issuecomment-720095343, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAU4VFFUC2LLYJVD6KCVTWTSNVUU7ANCNFSM4S5ZDO7Q .

mirh commented 3 years ago

Right.. I keep forgetting that detail

It runs when doing origin as admin though, which means origin may have changed their launch rules...

I can't be bothered to.. uhm, like convert my steam installation to origin Though if it's like just any other games I have checked, file __Installer\installerdata.xml should contain all launch parameters and whatnot.

Speaking of which, did you check whatever you had to test about RUNASINVOKER?

Mgamerz commented 3 years ago

I have not yet test RUNASINVOKER. Been really busy. I'm wondering if the user did not have admin rights so the elevation service was not running or something. Origin seems to automatically elevate processes that need it. It won't run an unsigned/invalid signed exe as admin, so I'm not entirely sure what broke, since there's no app crash.

Anyways, I'm finally getting a more full understanding of this. You've done some good research, I tested you permissions trick and that worked. It will take some trickery to get it to fully work in the installer (and I found M3 also shared the V3 AGEIA key code, so it'll have to be fixed there too).

I did a bit of digging around in MassEffect.exe and found the subroutine that sets these keys. It doesn't even check if they exist. It always tries to write them. I'm wondering if it would be possible to patch out the check with an asi mod since it in theory would run before PhysX loads (binkw32.dll loads before PhysX, according to IDA's module loading log). I patched it out in a non-origin executable and it worked, but I could not patch the origin executable as it seems compressed/encrypted. The exe is nearly the same size, but the content of the origin one is only half. however so I think it has a big gap at the end so it can map the decrypted contents back to the original offsets in the loaded process memory.

I don't really have time to investigate this further, but it would be something like https://github.com/Erik-JS/masseffect-binkw32/blob/master/ME3/main.cpp and how it does a virtual protect. The call in a non origin executable is at 0x8B7427, which starts with 0xE8 (call). Writing 6 0x90's here nop'd out the whole call.

After this mad rush of things is over I might take a poke into seeing what I can do about this. It's hard because you have to deal with users who may or may not have the patched no-admin exe, might not have local legacy physx, and I would like to avoid a scenario that busts things in any of them.

Mgamerz commented 3 years ago

I'm going to bundle SubInAcl.exe to set the ACL for me on this registry key. I have my permissions granter exe, which grants write permissions to directories, but I don't really want to spend the time to extend it's feature set right now. I'm wondering if instead of converting permisisons to explicit (instead of inherited) for this key, we just add a Deny permission to Create Subkey.

image

Deny always takes precedence so the only change would be adding to the ACL. I assume not having the permission does the same thing. I do wonder if this breaks vanilla games though.

Mgamerz commented 3 years ago

Just going to bounce an idea off you. I'm going to package this into a cmd script so I can do only one admin invocation for legacy PhysX.

REM This script requires admin rights!

REM Install PhysX Legacy (installs the msi)
msiexec /i /qb "%1"

REM Remove enableLocalPhysXCore
reg delete "HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\AGEIA Technologies" /v enableLocalPhysXCore

REM Remove EpicLocalDLLHack
reg delete "HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\AGEIA Technologies" /v EpicLocalDLLHack

REM Prevent ME1 from installing keys to this directory (which breaks games like Mirror's Edge)
subinacl /subkeyreg "HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\AGEIA Technologies" /deny=administrators=C

Assuming PhysX legacy is installed, I think that ME1 should still run, even on a vanilla setting. It will run as admin and should have it's permissions denied, in which case it will fail over to the system PhysX.

Mgamerz commented 3 years ago

Hmm. Setting Deny worked, however it makes re-installing not work, because the installer doesn't have permission to write registry keys in the directory. I don't know if there is a way to work around this issue.

mirh commented 3 years ago

It will take some trickery to get it to fully work in the installer (and I found M3 also shared the V3 AGEIA key code, so it'll have to be fixed there too).

I'm not sure what V3 is, though of course later MEs would need 2.8.0 and 2.8.3. Bioware didn't seem to slack off with redists at least then.. But I guess you could have a check for those prerequisite too (if the games are installed).

It doesn't even check if they exist. It always tries to write them.

Yes, that's what procmon showed me too. I noticed that when I'm not running it as administrator though, when it gets access denied on RegOpenKey(write) and/or RegCreateKey(Set Value, Create Sub Key) it doesn't even *attempt* the remainder BS.

I'm wondering if it would be possible to patch out the check with an asi mod since it in theory would run before PhysX loads

If you could tell me after 7 years what the hell each value does specifically, I'd really be glad... Though even though clearly the more "targeted" the fix, the better usually, if you can't pull it off for all games editions it doesn't really make sense to have such kind of.. variability.

I'm going to bundle SubInAcl.exe to set the ACL for me on this registry key.

Remind me again why you can't set ACL from .NET, or at least call native functions? I'd also be damned if microsoft didn't have put the more graceful admin elevation system of the world in their own programming language.

I'm wondering if instead of converting permisisons to explicit (instead of inherited) for this key, we just add a Deny permission to Create Subkey.

At least on W7 I had to disable inheritance to make whatever custom setting to stick tbh. And once I did that, I couldn't get a separate set of "grants" and "denys" for administrators (notice how you just completely overwrote all permissions for the group in your screen, and that's what fucked up your physx reinstall)

It will run as admin and should have it's permissions denied, in which case it will fail over to the system PhysX.

I guess you are rushing your ass out for saturday, but I hope V4.1 won't have the stupid admin thing anymore :)

Mgamerz commented 3 years ago

I can call it from .NET. However, that would require launching a new executable. I could relaunch ALOT Installer... but that's not really fast (since it's a fat exe due to how .net core is designed). While I can edit the ACL in .net, I also need to run PhysX as admin and a bunch of other stuff. Doing it in an external file is just easier since I'm shipping an external file anyways.

And yeah, good point on the pic. Between that post and now I found that out. Powershell has some decent ACL commands that let me target non-inherited permissions (so my new grant or deny), I'll just call that script within my PhysX setup script. and yeah i need this done ASAP because we're going to be bundling this installer into ALOT's download, so I need a version that works, in the event that the user can't update (which in some countries happens, like china not talking to github).

V3 is the current ALOT Installer. V4 is the new rewritten one.

mirh commented 3 years ago

but that's not really fast (since it's a fat exe due to how .net core is designed)

Mhh, I see. So if restarting the very same program is inefficient, you either run another one... or I was actually told that you can do some magic with COM?

https://stackoverflow.com/questions/127042/how-to-uac-elevate-a-com-component-with-net https://docs.microsoft.com/en-us/windows/win32/com/the-com-elevation-moniker

Doing it in an external file is just easier since I'm shipping an external file anyways.

Mhh.. Even though the installer is its own ship, I guess like a single admin request to rule them all ain't bad though.

Powershell has some decent ACL commands that let me target non-inherited permissions (so my new grant or deny)

Well, if you can fix that perfectly (i.e. the only thing changing is just "create sub key" for that very specific key) I guess that has the precedence. Though .NET shouldn't be half bad either AFAICT https://stackoverflow.com/questions/2151074/setting-registry-key-write-permissions-using-net https://docs.microsoft.com/en-us/dotnet/api/microsoft.win32.registrykey.setaccesscontrol?view=dotnet-plat-ext-3.1

Mgamerz commented 3 years ago

My application is signed, so it goes through authenticode on boot. This takes typically 1-2s. SubInAcl is about 200KB and is not signed... but i'm seeing it's not applying a new entry in the ACL list, it's replacing inherited instead. I'm not sure how that works since it should be inherited already. I'm seeing if it can be done through powershell to clear any non-inherited ACLs (which would 'restore' full controls), and optionally add a Deny on CreateSubKey. I can access the granular entries, but for some reason it's consolidating them when I set the ACL.

Doing it manually though, I can achieve what I want. I think I need to make sure it only applies to this key. That way the inheritance flag is not the same, so maybe it doesn't try to consolidate it: image

I think I will also add a new REG_SZ that describes the change that occurred. In the event someone has problems and goes into the registry to investigate, they can see what I did.

Mgamerz commented 3 years ago

Also - you mentioned create subkey. Are we sure this isn't meant to be SetValue? Creating subkeys is for creating sub'folders'. The game is calling SetValue. See the bottom block.

image

The middle block I believe creates the key if it doesn't exist. The bottom one tries to set the keys. I'm in ACL hell right now with windows denying me permissions to set permissions.

Mgamerz commented 3 years ago

I just tested with SetValue denied. Assuming Legacy PhysX never gets updated (not sure how it would?), this fix should work. The only issue I see that it will probably interfere with other games trying to install PhysX (like a forced reinstall). Unfortunately there's not really a way around this because we either allow it or we don't.

I've built this into V4's latest code. In the redistributable package I give admins ownership of the key and strip non-inherited ACLs (giving them permissions. this removes the deny entry if we're doing this again). Then I install PhysX. I then strip ME1's two values it adds, add a note value, add the deny permission for SetValue, then restore the original owner (if it's not administrators originally).

Still kind of worried that repairs of other games might not work. I might make a reversing release that removes the Deny permission that we can distribute if users have issues. How users will know it's ALOT is a whole different story, I doubt many will understand if you explain it to them.

mirh commented 3 years ago

I know SetValue "works" for what we are trying to do, even though it causes a waterfall of undesired effects, that's why I was kind of giving up to the idea of fixing it for good. But as I was saying, with procmon I found out that the game isn't getting fixed through the "SetValue" operation being blocked... we are talking about the parent RegOpenKey and RegCreateKey failing, which in turn stops the game from doing anything else with that key. And, after some extra tinkering, I found out that denying "create subkey" is enough to trigger this, while still allowing all values to be updated as desired.

Idk about the disassembly then, and you can even uninstall and reinstall physx with this in place.

p.s. I don't think I could have written a better dialog, though I'm not really sure we are talking about something so crazy life critical that it deserves a CAPITAL LETTERS WARNING.

Mgamerz commented 3 years ago

I've learned if you don't write it in caps, the user probably won't read it. And there's like a 80% chance they still won't. But you can't blame me for trying to make it not obvious :)

I'll test out CreateRegKey again. Maybe I misread the disassembly and forgot that opening keys with permissions also can be denied.

mirh commented 3 years ago

Or maybe you still had some fuckup with inherited and not inherited permissions, and whatnot. It's really a mess.

Mgamerz commented 3 years ago

I changed it to CreateSubKey and it also worked.

But I also removed PhysX entirely (all versions), checked there was no NVIDIA CORP folder in my program files, entirely deleted the AGEIA key... and the game still ran. It didn't create the key either. So I have no idea what's going on. I don't know if we even need to actually install LegacyPhysX if the game just loads the local one anyways (maybe it does that if it can't find the system one?)

Orrrrrrr it's because windows is dumb and did not refresh the registry view. The keys were created. Blegh.

Uninstalling legacy PhysX will wipe out the keys. However, if there's a value added not by the installer, the key remains. So adding the note value almost seems necessary to ensuring the permissions are always correct on the key.

Mgamerz commented 3 years ago

Okay, more testing:

image

There is no local Legacy PhysX installed. The deny permission is set on this key for CreateSubKey/Adminstrators. image

The game still runs. I don't get it. Does it not actually depend on system physx without those values? What the hell are the point of these values then if they don't?

Also, setting this deny subkey makes legacy physx installation not work. image

You can run the installer in repair and it works. If you uninstall then reinstall, the above message is thrown.

Mgamerz commented 3 years ago

image

This with our registry deny rule, the enableLocalPhysXCore /epicdll values not present. It still loads the local one, even with PhysX installed.

This key seems to be read from PhysXLoader.dll. image

I don't see any references at all to EpicLocalDllHack in either PhysX file.

mirh commented 3 years ago

I don't know if we even need to actually install LegacyPhysX if the game just loads the local one anyways (maybe it does that if it can't find the system one?)

Absolutely not. That's why the key fucks up everything so much.

There is no local Legacy PhysX installed. The deny permission is set on this key for

Something must be corrupted about your installation in the first place, because AGEIA should have two subkeys. Permissions should only be denied after a working installation is in place.

Then user can even uninstall and reinstall (keys will never be deleted, but they will be empty at worst).

This key seems to be read from PhysXLoader.dll.

That makes sense. The switch in itself would perfectly make sense for a developer to test this or that. The crazy shit is why the hell the unreal engine went at such lengths to avoid shipping an extra installer.

It still loads the local one, even with PhysX installed.

.... Ok this is really unexpected. The game should be querying PhysXCore Path and following it. I cannot seem to reproduce this.

Mgamerz commented 3 years ago

My enableLocalPhysXCore variable is set to 00 00 00 00 00 00 (edit: lmao using a vpn causes this). Not sure why. And I had no registry keys to begin with, so it's the reinstallation trying to set values that is not working. I think I have a way we can simply force Mass Effect to always use local PhysX. Then there would be no need for an installation of PhysX to occur.

image

I disassembled the loader and after an hour of banging my head on the wall I think I've worked out the function. It's doing the mac comparison. If I just nop out the jnz short MacAddrByteNotEqual it will always think the mac is the same, regardless of whatever value is there. This will make it think the local PhysX core file should be used.

I disassembled it in Ghidra as well. Was really helpful to figure out some stuff. image

I think we can patch it out. Unfortunately that doesn't fix the game trying to write the registry key to begin with, but this will make sure Mass Effect always uses the local version so we would not need system PhysX.

Then user can even uninstall and reinstall (keys will never be deleted, but they will be empty at worst).

This is not true. I have observed the keys being deleted. If I install legacy physx (after cleaning out all AGEIA keys), then run the uninstaller, the AGEIA keys are gone.

If you want to simulate the installer being broken, install legacy physx, let mass effect install it's values, then put the deny permission in place. Then install PhysX and try to install it again. The leftover value ME left behind will keep the deny subkey in place (otherwise it would have just deleted the AGEIA key entirely). The installer will not be able to install the subkeys, so it doesn't work.

mirh commented 3 years ago

And I had no registry keys to begin with, so it's the reinstallation trying to set values that is not working.

Because you probably screwed up permissions, thus glitching the installer. I totally verified on a clean system legacy should create everything as I said.

This will make it think the local PhysX core file should be used.

That's definitively one spin I didn't foresee. It's quite workable then.. But to be absolutely honest I'd rather condom ME1 doing the screwing.

I have observed the keys being deleted.

If your permissions are like normal, sure. But there's no way you can delete keys if "create subkey" is denied. Ok, I admit that for some reason I was thinking the adjusted permissions were keeping subkeys in place.. Instead it was just leftover values. The installers outsmarted me I guess. Anyway, then.. The trick is just not having any leftover value?

Mgamerz commented 3 years ago

The problem with that approach... is Mass Effect writes a key into it if it is given the chance, which seems possible. I can only control what happens during the install. I can't see what happens outside of that session. We can add deny but then we're trading one problem (messing up games) with another (potentially making those game's installers fail). I think messing up the game, which is easily fixable (given the knowledge), is a better choice than making the install fail if the PhysX install fails.

I know your issue is that our installer is the one making the issue, but the issue seems like it's Mass Effect itself, not the installer. The old fix of making it writable just kind of made it work by letting ME install it's keys without admin (that it would install anyways if it was run as admin). I'm not saying I'm giving up on a solution but I don't think this is an issue due to the installer (v3).

The real issue is how do we make Mass Effect run without actually requiring writing into that key. I know how to fix it (I can see the lines in the assembly) but the problem is that Origin has an encrypted executable. I can't patch that part the game sitting on disk. On retail (which is DRM free) I can patch it out, though I haven't tested this cause there's no point since the whole problem is due to origin. It might be possible to do a virtualprotect and modify the game in memory through an asi (or the bink loader) but that's beyond my skillset right now. A try catch was just too difficult for Demiurge I guess.

I'm not really sure how to approach this problem - we can force Mass Effect to use local PhysX, but the key is written by the executable, which is not reliably patchable. Only some of the Origin version executable seems encrypted/compressed - the enableLocalPhysxCore string is not. We need to edit ME1's executable to give it 4GB of memory to use, Origin won't launch it if it's not signed as EA and requires admin... So, we must address the real problem: We need to find a way to actually let Mass Effect run without requiring admin access - preferably, by just making the game executable not write those registry keys at all.

I edited the enableLocalPhysX string in the Origin executable and it wrote the new value name. I guess until we find a way to patch the executable or process, we could do AGEIA writable + edit the name of the value it writes so that, at least our installs, don't write the enableLocalPhysXCore value, plus we could strip out the enableLocalPhysXCore on install. We could also use the force local PhysX. I don't think there is time to find a better solution that doesn't goof up other game repairs. (Lord knows we need someone at EA/another company to debug a repair gone wrong on their digital platform for some old game and find out it's us messing with stuff).

So I guess we don't need to install Legacy PhysX after all though, so that's a plus. Not really sure what kind of solution I'll come up with in the next few days though...

mirh commented 3 years ago

I know your issue is that our installer is the one making the issue

I mean, now it isn't anymore thankfully. Anyway, I get your... very special.. concern, for users.. to eventually always find a way to fuck up everything But (hoping we did settle that uninstall and reinstall of physx should work?) you shouldn't loose your sleep on people "literally removing the game freaking prerequisites".

On retail (which is DRM free) I can patch it out

I thought that had securom?

We need to edit ME1's executable to give it 4GB of memory to use, Origin won't launch it if it's not signed as EA and requires admin...

Mhh, one problem at time please. :)

We need to find a way to actually let Mass Effect run without requiring admin access - preferably, by just making the game executable not write those registry keys at all.

It's not really about "admin" or "not admin", because UAC is just a security feature and some people can even disable it (also, windows XP out of a showerthought) That's why I'm trying to stick to the greater picture of formal correctness than just getting the damn thing to work. You def should stick to the principle of the least privilege, but even if you don't that should come with break-consequences for other games.

... Anyway, how about for the greatest damn peace of mind in the galaxy, you edit PhysXLoader.dll and the createkey permissions (hoping we can understand why your procmon was different than mine)? It's the most crazy resilient thing I can think of.

Lord knows we need someone at EA/another company to debug a repair gone wrong on their digital platform for some old game and find out it's us messing with stuff).

Hoping they aren't as shitty as microsoft's tier 3 business support :)

Mgamerz commented 3 years ago

It's not really about "admin" or "not admin", because UAC is just a security feature and some people can even disable it (also, windows XP out of a showerthought)

It is about admin or not admin. We have to modify the executable for 4GB, or ME1 will crash pretty much at the main menu with textures installed. Origin will not run the game because of the requirement for admin, which is by the Windows Compat DB, I assume because of these assumptions of admin rights ME has. Dropping the admin invocation is possible, but it doesn't work around the new issue that now it doesn't have permission to the registry key. Origin not running the game at all is not a solution. The existing solution is to just let Mass Effect write the key it needs, that it would write anyways if we didn't patch the executable (since it would run as admin).

Also, the issue is not UAC. XP supported principle of least privilege too, though not as much as UAC. Could you even play Mass Effect on XP if the account wasn't in the administrators group?

I'm looking at maybe one more idea. The strings table in all of the executables appear to be viewable, and the Software\AGEIA Technologies is used in two nearly back to back calls (same value though). I'm wondering if we can modify only one of them to have it essentially have the same effect as when the deny permission is in place. I can't edit the length unfortunately, so that's incredibly limiting. I'm going to attach a debugger and see if I can see how it's behaving.

Mgamerz commented 3 years ago

Actually just tested some stuff. I'm not actually sure why mass effect crashes without admin rights. I assumed it was the registry writing... but that doesn't seem to crash it. Maybe it was cause localPhysXCore wasn't set - but that also doesn't seem true either as I don't have it set and the game is still loading. But ProcMon again is now showing it's loading the system PhysX.

image

It's like they wrote if (random() == 0) in their code, I swear...

However, this is useful to know. I uninstall system PhysX and now it crashes. Conveniently the PhysX SDK has exports so I can see some context of where it died: image

I'm going to guess it crashes without admin only in some instances. If the key doesn't exist - which it won't on first run - the game won't run. That may explain some of the crazy inconsistencies - it's only the FIRST run that has to be done. You said this earlier in some of your posts but I get it more now. I swapped in my patched local PhysX loader and huzzah! It works!

That means we might not have to actually change any permissions at all. The crash is because the value was missing. It couldn't find the library to load, so it died. Forcing it locally means that value doesn't matter. Knowing that I could just change the string name of the value it's going to write in, so our modified executable of Mass Effect won't install this key. I can't fix it for other installs (especially if ME was run before - but I can strip out the key) - but that looks like it might be a workable solution. Our patched ME won't install the value that breaks games (it'll write something else), it will always use local PhysX, and we won't have to mess with permissions.

mirh commented 3 years ago

Origin will not run the game because of the requirement for admin, which is by the Windows Compat DB, I assume because of these assumptions of admin rights ME has. Dropping the admin invocation is possible, but it doesn't work around the new issue that now it doesn't have permission to the registry key.

Writing that key is bad regardless of the admin requirement And the admin requirement is bad regardless of the key

Also, the issue is not UAC. XP supported principle of least privilege too, though not as much as UAC. Could you even play Mass Effect on XP if the account wasn't in the administrators group?

My educate guess is no. Though you should remember that patches comes with VistaFirstRunFix.exe, which just took care of this aspect (so they were well damn aware of the problem.. maybe they even had some kind of fix in the official installer? and then you also have runme.exe)

Though anyway, what I'm saying is that neither normal user nor mad people disabling UAC should be left out.

It couldn't find the library to load, so it died.

Mhh.. no shit? I'm not sure when this wasn't apparent. (in fact I was probably tipped off about the need of legacy physx by the actual dll returning NAME NOT FOUND) Anyway, presuming you have handled whatever mess of conditions you had been having, good job with this kind of self-contained fix. I'd still appreciate if the AGEIA condom could also ship though, for an added layer of security should integrity verification trigger or something.

Mgamerz commented 3 years ago

We are patching the exe to write a different named value than enableLocalPhysXCore. I'll strip out the leftover keys as well. This way the files produced by alot installer will not corrupt other games.

But like I've said all along, this is not ALOT Installers fault. This is Mass Effect's fault. And because of that, this implementation is more of a convenience for you than it is for actual users.

With our new solution we don't need to change the permissions at all, and by default I'm not going to, since the issue is not really in the scope of this project any more than it would be for mod manager. It is not the installer's job to defend other games against me1's bad code - we can prevent it on our patched installs but i'd rather not mess with deny permissions to try and 'fix' people running the non patched game by default.

I have an install script package that does all of the fixes already for the containment of mass effect writing the values. It is easier to just ship that standalone. It's what I wrote before investigating the actual crash. Wrap it in a self extracting exe and boom you have a way to patch it without the installer or doing things manually while ensuring it runs as administrator.

Additionally, Iget the feeling you're the person who seems to like the PC gaming wiki - like when my fix for AMD lighting conveniently got removed moments after the dll fix came out, even though the new one doesn't work for all users. The fix I made isn't even listed anymore even though it fixes the problems for those users.

And for some reason alot installer is blamed on that wiki as breaking other games (I assume this was you, since you are the only person I have ever seen care about this issue) when this is still Mass Effect's fault, not ALOT Installer's (but we get listed as being blamed anyways for some unknown reason). ALOT Installer does not write the values, the game does. Even with the permissions granting we did, it is still the game writing the keys like it would be doing without the installers modifications.

Screenshot_20201103-065625.png

This fix was already pretty awful to try and develop. Physx is terrible. It might have good physics implemention but the installer system is total garbage.

mirh commented 3 years ago

The installer isn't that bad (even though it could have seen many improvements, as appened with 2.8.4+). Whatever the fuck UE is doing here is nuts if any. ... I wonder what you are missing out by sticking to 2.7.2.5 rather than .9 though 🤔🤔

Additionally, Iget the feeling you're the person who seems to like the PC gaming wiki

I'm actually a mod there, that's why I'm kinda bending everywhere for the best universal solution. And yes I did all those changes. I had forgot to add back your fix after discovering https://github.com/CookiePLMonster/SilentPatchME/issues/3 though, sorry

And I get that the ALOT note isn't telling 100% of the story, but I'm always striving for succinctness to get down to ELI5 levels.. if I can explain (also your harshness in #17 didn't really help my tender)

We are patching the exe to write a different named value than enableLocalPhysXCore. I'll strip out the leftover keys as well. This way the files produced by alot installer will not corrupt other games.

And that's cool. But besides the aforementioned concerns about other games, I was suggesting you to still keep the registry fix in order to have even more layer of protection against this crap. Like, the moment steam/origin was to verify integrity the game would become unbootable, isn't it?

Anyway, if you don't feel like, I don't have hard grudges for its inclusion after the magic you already pulled off. We can come back to this in the future, when you'll find time to dedicate to RUNASINVOKER

Mgamerz commented 3 years ago

As far as I'm aware, Origin doesn't write the key, but I haven't checked recently either. I don't have ME1 on steam so I can't check that. I'll test origin repair today.

If Origin repaired the game, it would still remain bootable because the exe would be restored. It would then require admin, which would make it install the key again, which would make the game work (breaking other games with enableLocalPhysXCore). I don't really think there is a 100% solution to this that doesn't have some side effect - breaking games or possibly breaking installers. I'd rather be the party that doesn't break anything though :)

In fact, if repairing it does not install the key, repairing the game would actually break the game under certain circumstances - the local dll fix wouldn't be present, so it would try to use the registry lookup. Since we would be denying it write permissions (assuming that fix was installed), it wouldn't work unless physx was also installed... Though, I guess that's a given if the user ran the physx install package/permissions deny for create subkey... But you can't really trust users 💯

When I'm done with the V4 and N7 day stuff I'd like to write up a research blog post about the PhysXLoader.dll change. I would love to write a story about the terrible code I had to reverse engineer. It's like the first real x86 disassembly I've done that actually lead to something useful.

Mgamerz commented 3 years ago

Just checked. Origin repair doesn't install the registry value. I assume it doesn't use the same patch system that the retail versions did, and they either a) didn't know about this or b) don't want to write an implementation that might differ from what the game did.

mirh commented 3 years ago

Of course, it's not like EA cares for their games. I think it's probably already a miracle if they managed to convert it to their native content delivery system (as opposed to the old one I remember BF2142 was using that was "just download a DVD image and run its normal installer")

I don't really think there is a 100% solution to this that doesn't have some side effect - breaking games or possibly breaking installers.

I don't think that you confirmed to me yet the two oddities I was left baffled about: a) enableLocalPhysXCore is all there is to (normally) control the loading path b) if there are no extra keys in AGEIA, the uninstaller will happily uninstall even with denied subkey creation

Since we would be denying it write permissions (assuming that fix was installed), it wouldn't work unless physx was also installed...

Of course the implication was that legacy physx was still getting installed. Anyway, just FIY for the future, running as admin isn't just bad from a "theoretical" pov, or for the key breaking other games It breaks all kind of things, from steam overlay and controller, to other injectors But we can get over this at another time.

p.s. big respect to any work and tutorial on disassembly

Mgamerz commented 3 years ago

Yeah, I don't like running as admin either, it's why I discourage it's use in ALOT Installer beyond necessary elevations like game directory permissions.

As far as I can tell, a) This is at least 'one' way of doing it. From the disassembly I did, the loader (at least, this version) appears to look for the value before loading the library. If the value matches, it tries to load the local one - I'm not sure if it actually checks if it exists. I mean, it doesn't check if the system one exists either... But it's not like it tries to load the local one then the system one.

It seems to call this method twice, once for PhysXCore and once for the cooking library - though, the one in the system PhysX has a different name (the local one is NxCooking). I'm not sure what the cooking library is for, since Cooking is a thing done at compile time in Unreal. It could be that the dll was included because they didn't "CookedPCConsole" ME1. I modified the 'ShouldUseLocalDll' method so it didn't matter how many invocations it had though, just ensured it behaved like it should for locality.

The real moral of the story is just all around terrible design.

b) It should uninstall and remove the keys, yes - I'm not sure if this is a PhysX installer behavior or if it's a Windows MSI design that empty keys are removed. I'm not sure if later PhysX installers touch this key or if they moved it. It's hard to know that everything that writes into this registry key though, that's a very broad question that we'll never know the whole answer to. It baffles me to think that AGEIA thought this was a good design decision, it's not like the PhysX library is very big at like 5MB.

In case you're curious, I wrote a guide to assembly hacking Megaman Battle Network 3 (White) on GBA. It's quite detailed and took me like a week to write. https://forums.therockmanexezone.com/intro-to-asm-modding-hooking-t5374.html

While it's a whole different and simpler instruction set, the concepts for it are almost 1:1 the same for x86 disassembly. It's one of my prouder works.

mirh commented 3 years ago

I'm not sure if later PhysX installers touch this key or if they moved it.

Newer physx installers make approximately the same operations. If/when nvidia will update it again, unless they decide to create out of the blue a new key, it should work perfectly.

It's hard to know that everything that writes into this registry key though, that's a very broad question that we'll never know the whole answer to. It baffles me to think that AGEIA thought this was a good design decision, it's not like the PhysX library is very big at like 5MB.

Redists weren't all that uncommon back then. I can think to OpenAL, visual studio and directx itself for example. ... and in fact I just tested UT3 (which shouldn't use that much newer engine build than ME, at least judging by the physx version attached) And physx always seems to be loaded locally. If you start the thing with the usual admin/write permissions, it will eventually check for the EpicLocalDLLHack value.. but as always god knows what that even changes.

Mgamerz commented 3 years ago

I disassembled the MassEffect.exe executable and the PhysXLoader.dll and the only time EpicLocalDLLHack appears is in ME, and it just writes it. It doesn't ever read it. I wonder if they put this value there specifically to prevent the key from being deleted on PhysX uninstall.

I don't have a problem with redistributables. But it shouldn't remove older versions (or break forward versions). I had a user uninstall Legacy PhysX when testing yesterday and it broke Mass Effect 3 because it removed some stuff that it shouldn't.

mirh commented 3 years ago

There's just two physx branches to install, and AFAIR each respective new version should be able to install on top of the previous one. One is effectively an old snapshot of the other though, so that's why you got that result I guess.

It doesn't ever read it. I wonder if they put this value there specifically to prevent the key from being deleted on PhysX uninstall.

I really don't think so. I don't see why anybody in the world would care about that.. except madmen trying to workaround shitty coding. The reasons probably go back to the dawn of UE3 (and its physx integration). I wonder what Gears of War, Rainbow Six: Vegas and Medal of Honor: Airborne did.

Mgamerz commented 3 years ago

I talked to an ME1 dev and they didn't seem surprised in the least. They had a lot of hackjobs and I wouldn't be surprised if this was Epic's 'we'll dedicate 30 minutes to a fix for you' thing, since it sounds like they didn't have time to spend on their partners.

mirh commented 3 years ago

Ehrm.. I wouldn't really be so quick to exonerate demiurge I don't know how the other early games are working, but I'm pretty sure they all ship with a redist like UT3, and that they have no stupid "plz runme" utilities.

Mgamerz commented 3 years ago

Nah Demiurge still has like another 50 tallies on the board for things they broke in ME1.

mirh commented 3 years ago

Or maybe they were broken on xbox already!

Mgamerz commented 3 years ago

I'm going to close this issue for now. However I plan to do a follow up once V4 ships and I write the article.