[SECURITY FIX] Profile import RCE

[sadkris]

Hi! Whenever importing a profile from a code or a file, r2modman would arbitrarily extract all files without checking the type. This allows the user to add DLL files to the modpack that aren't included on the mod list, allowing a malicious party to add malware that is automatically loaded by BepInEx and run in the game without appearing on the modlist of the pack. This change ignores DLL files when extracting, not allowing them to be extracted to the user's installation directory.

Suggested remediation in the future would be to add more security onto the profile code APIs, as any user can upload any data permanently and receive a profile code that can be used to download their arbitrary content from your servers. This can be used in many malicious ways, for example allowing a hacker to shift blame to you for holding exploit code on your local servers.

[CLAassistant]

All committers have signed the CLA.

[sadkris]

@ebkr Bumping this since the longer this is left in the public eye, the bigger the security risk. This is already a really critical vulnerability

[MythicManiac]

Hey, just commenting to acknowledge we've seen this. Right now this isn't considered urgent due to the fact every & any mod install in itself already counts as RCE, so the main difference here is that this isn't visible in the UI the same way normal mod installs are. In other words, this vector isn't as major of a problem as it might appear on the surface if you consider it in the context of the entire ecosystem.

We will most likely address this but it'll require narrowing down the profile import/export file format & feature capabilities somewhat. The way it currently works is by design and there's some chance we'd break something by simply merging this change as it is, although at least based on some preliminary investigation it looks like it shouldn't break anything important.

[sadkris]

Hey, just commenting to acknowledge we've seen this. Right now this isn't considered urgent due to the fact every & any mod install in itself already counts as RCE, so the main difference here is that this isn't visible in the UI the same way normal mod installs are. In other words, this vector isn't as major of a problem as it might appear on the surface if you consider it in the context of the entire ecosystem.

We will most likely address this but it'll require narrowing down the profile import/export file format & feature capabilities somewhat. The way it currently works is by design and there's some chance we'd break something by simply merging this change as it is, although at least based on some preliminary investigation it looks like it shouldn't break anything important.

I disagree with this. By nature all mods have a risk of RCE, but the point of this issue is that it allows people to hide mods inside the export code without a user's knowledge or ever being listed in the UI or store