PluginCompiler.Whitelists - existing Whitelists are too narrow

[nikita-tukkel]

When writing something more complicated than “hello world”, it could be great to use the most of .Net core libraries. The existing whitelists prohibit too much. For example, I can’t use Type.IsAssignableFrom and have to do an ugly try-catched cast instead.

I propose to replace Whitelists with Blacklists, prohibiting things useless / not compatible with Unity.

[nikita-tukkel]

Another obstacle example - not whitelisted UnityEngine.Profiling.

[Erdroy]

Yeah, that is true.

Could you write down a proposal list or PR for that? I don't have free time at the moment, and not sure when it will change, so at least maybe I could add these namespaces/types to the whitelist.

Also - I'm thinking of a popup, where if mod fails to compile due to sandbox issue, the popup would show what code it failed at - and allow the player, to allow it to compile anyway.

[Omegapol]

IMO more responsible way is to still use whitelists, we can't expect user will know what he risks and what he is doing, and that will lead to him/her blindly accepting everything.
Perhaps that option could be hidden behind debug startup option so mod devs can keep working on while request for a whitelist item is sent.

[nikita-tukkel]

Could you write down a proposal list or PR for that? I don't have free time at the moment, and not sure when it will change, so at least maybe I could add these namespaces/types to the whitelist.

It is understandable that you have more important things in your life to do, and I believe there is no need to excuse for that. You already did a great thing using your own free time. If you will have inspiration to do more, it is great. If not, well, this is how things go in real life.

I think I should be able to code a PR for that if only I have an ability to build the project. At this moment I don’t want to install Visual Studio - this bloatware always torturing me and it is too much for a small hobby. If only there is a console build possibility. I created Issue 23 for this.

Proposal is something like below; choose how you like to see it, depending on your vision:

Minimal option

Whitelist: System.HashCode, System.IO.File, System.IO.Directory, System.Type.* and ALL the related classes like TypeInfo, DirectoryInfo, etc.

Whitelist all packages from Unity.*, at least Unity.Profiling and ALL the related classes.

Median option

Remove Whitelists for System and Unity, just allow everything.

Don’t worry too much about the security: with Harmony and with the ability to publish DLL-based mods security is already screwed, anyway.

Maximum option

Implement Blacklists to block stupid things. E.g. to block things from System that has more suitable replacement in Unity. Can’t come out with a good example, though. First, I wanted to propose to blacklist System.Threading.ThreadPool because Unity has IJob, but I don’t see the reason - I better assume that the mod developer knows what he is doing.

[Erdroy]

The idea of sandbox is to limit any harm that the mod can do, so that is why System.IO.File and System.IO.Directory are blocked.
It a mod needs access to IO, it probably wants to store/read some config etc. and that can be accomplished with an API in Addons (PluginStorage). Otherwise, we can't predict, what the mod will do with the access to IO, it could for example, add something to autostart, destroy the registry etc.

TypeInfo should be safe enough, it is read only - but anything that involves invoking methods via reflection is a no-go, it would be a way around the sandbox.

Also, I do agree with @Omegapol, a way to allow prohibited code to be compiled, is a bad idea. Players would just blindly click that "Load anyway" and... yeah.

[nikita-tukkel]

If you want to prevent disk writes, then you should also block System.IO.StreamWriter and probably many other things. Even with most of System.* blocked, I believe a curious person will find a way to download and inject his DLL using only Harmony and Unity internals.

[nikita-tukkel]

As for reflection, you already have HarmonyLib.Traverse which do the same thing. And it is good, because I use it to access some private fields in Stationeers code. Even without Traverse, you will not block HarmonyPatch, which gives you access to __instance of anything, and further without reflection.
So my point still as I wrote above - it is too late to care about security.
If you want a serious sandboxing, it should be implemented on the level of .net execution environment. I don't know if it has such mechanics, but probably it does.

[Erdroy]

__instance will be blocked, because harmony patches also do go trough sandbox.
dotnet core has something like this, but the game is running on mono - so the only way would be to make a separate AppDomain with security flags, but that would way to much work, as everything would need separate API and it would be really slow.

The idea of sandbox is to limit the ease of doing something suspicious and to evolve over time (but I guess, no one wants to help with it, so...).

I have a mod I'm making that will enable using non "Default" starting conditions used by custom DifficultySettings.
This mod overrides the async void WorldConfigurationMenu.StartGame() function.
With the whitelisting, I'd have to use BepInEx and basically scrub the idea of publishing it on the workshop.

These are blocked because of the white/black listing:

UniTask
AsyncVoidMethodBuilder
IAsyncStateMachine
AsyncVoidMethodBuilder.<any function>
^^ These are all under Cysharp namespace (UniTask.dll , UniTask.*.dll)

Func<bool> (really any Func<>)
UnityAction
UI.ConfirmationPanel

The UniTask*.dll's aren't referenced either.

A suggestion is to have a custom whitelist file in the Scripts directory, with assembly references.
Possibly the best would be a xml file, with:

<References>
  <AssemblyName>
  </AssemblyName>
</References>
<WhitelistTypes>
  <type></type>
</WhitelistTypes>
<WhitelistAssembly>
  <Assemblytype>
    <type></type>
    <excludedtypes>
      <type></type>
    </excludedtypes>
  </Assemblytype>
</WhiteListAssembly>
<WhitelistTypesNamespaces>
<BlacklistTypes>

Just for now: this post is just a starting point for the discussion with the whitelists.