unitystation / unitystation

The original unitystation
https://unitystation.org
GNU Affero General Public License v3.0
710 stars 633 forks source link

StationHub should provide some measure of security for untrusted builds #7158

Open NoooneyDude opened 3 years ago

NoooneyDude commented 3 years ago

In the future, we would like Station Hub to link to servers that host builds from a non-official fork. However, obviously, there are security implications with allowing Station Hub to download binaries from an untrusted source, and it is impractical to manually vet every single build from every potential fork before hosting on our CDN.

One solution to this problem is to move all dangerous calls (for example, file writes) to an external assembly. The game would then dynamically link with this library when these calls are required by the game. This DLL would be hosted on unitystation CDN and Station Hub would fetch from this trusted source.

In addition, however, Station Hub would need to scan builds from untrusted (non-Unitystation) sources and use something like ILverify to check all members against a whitelist of known-safe calls. This will provide a measure of protection against malicious builds or bad server operators. I assume this won't be perfect, but it will be a huge improvement over nothing.

If this process takes time, we can possibly provide an option to the user to trust this publisher and skip future checks (disclosing that it would be at their own risk).

It is important to note that we still want to allow builds to be run without Station Hub, so the game would probably have to ship with this supposed DLL too, but Station Hub will need to somehow (suggestions welcome) overwrite this DLL in favour of the one Station Hub acquired from a trusted source (our CDN).

I decided to post on the game repo and not station hub repo, because this gets more views and changes will be required in both, anyway.

Bod9001 commented 3 years ago

to add onto it,

more Technical specifications The Unity executable for Windows is generic across builds though not across Unity versions, so the hub could have a good known copy of the Everything but the unitystation_Data folder, And overwrite the clients content with that. this needs further testing and refinement tho

then it needs to, browse through the assemblies of unitystation_Data\Managed and overwrite the specified DLLs with the validated specified ones such as one is for mirror and other stuff that is dangerous, and then it should check the remaining assemblies for code that could be potentially used in a malicious manner such as reflection, file OI, that type of thing, since it should be done through the known good DLLs that are provided by the hub

same thing should be done with unitystation_Data\Plugins as well, Streaming assets as well,

https://docs.unity3d.com/Manual/WindowsStandaloneBinaries.html

1/2

Bod9001 commented 3 years ago

2/2

7160

This issue also applies to prefabs that are built as part of the game, so, the hub need some way of checking these / nullifying the issue for the inbuilt prefabs, and scenes

good news it doesn't seem to work with, remote content that's on a Web server, it doesn't seem to download or even run (might be missing it up)

though it does still run Local content so is still dangerous, the patch to the unity event class is the best option, and would fix this issue as well making it so it has to be nonstatic is a good first step

Bod9001 commented 1 year ago

https://discord.com/channels/273774715741667329/312454684021620736/1066362390402310144 "first load it for reflection only to scan, then reload for real, otherwise you can get fucked by malicious code within assembly/type initializers or use some shit like IL decompiler to transform assembly into IL code and scan that"

"well, then there's another possible vector of attack called "struct layout fuckery" "

"in both mono and CLR runtime you can fuck with objects and read/write anything into them if you manage runtime to load structure with fucked up layout, that makes value and reference type fields sit in the same place in memory in mono it makes executing basically anything really simple, you just fuck the come delegate code with shell code you've created beforehand I also don't khow why they allow MethodBase.GetMethodFromHandle"

"I also wonder how their assemblytypechecker works Because there's also ANOTHER possible vector of attack you can put code behind last return inside the method I don't recall what you do afterwards, but i just know that some IL decompilers stop if they see last possible return in method basically, making all code behind it invisible meanwhile, there's some other fuckery required to use it but it's still possible" "again, i'm for LUA "

Bod9001 commented 1 year ago

they had little go at it and weren't able to find any vulnerabilities but still Worth looking to