Open YuriSizov opened 2 years ago
YuriSizov
commented
2 years ago In a proposal review meeting we decided that a good idea for now would be to implement a "no tool mode scripts" mode when running unknown projects/when the editor previously crashed. It can be enough to prevent immediate attacks, without giving a false sense of security from a proper "trust" system.
We can also bikeshed later on a proper sandbox mode, and on scanning AssetLib entries for malicious intent.
Zireael07
commented
2 years ago @YuriSizov: Good idea, I've had my own tool scripts infinite loop on me a couple of times ;)
ellenhp
commented
2 years ago We can also bikeshed later on a proper sandbox mode, and on scanning AssetLib entries for malicious intent.
Excited to bikeshed about this later, but for now I agree with this approach. Might also be worth examining attack surfaces in resource importers.
Describe the project you are working on
This is relevant for everyone who opens projects created by others, be it MRPs opened by contributors or just helpers on Discord and other community platforms.
Describe the problem or limitation you are having in your project
Godot editor treats every project equally, but not every project is created equally. As Godot is a game engine which is inherently open to do both system-level and networking activities, projects can be made malicious. The ability to execute code in the editor itself using plugins and tool scripts means that malicious code can be executed the moment you open a project.
This doesn't bode well.
Describe the feature / enhancement and how it helps to overcome the problem or limitation
Taking an inspiration from VS Code, among others, we can introduce a trust system, where projects not created by a local instance of Godot would be untrusted by default, and projects created locally would be trusted.
A trusted project would function exactly the same as any current project does. Although, if this gets traction and further demand we can also think about adding granular flags (maybe as an Editor Feature feature) what is allowed and what is not.
An untrusted project gets a prompt at launch which informs the user about the project's state, suggests to pick either trust or continue in the untrusted mode, and maybe explains what's going on with that whole trust thing. An untrusted project would be limited in terms of potentially harmful operations, such as system executions, filesystem access, and any kind of networking. At least as far as user-generated code goes. Editor's built-in functionality should probably still operate, but we can discuss this further as that still provides some attack vectors.
Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams
Limitations for an untrusted project can be implemented on the class by class basis. As a high-level idea I imagine harmful classes can be embedded with editor-specific compile-time branches which would check project status before doing harmful staff. This would create weird dependencies, though, even if conditioned, so I'm open to other ideas on how to better sandbox it.
We can iterate on what things get locked out with later revisions, but what we should definitely include is:
res://anduser://(or readonly access outside of those, but it is still dangerous).OS.execute,OS.shell_open,OS.kill(at least those, there are a bunch of other methods that expose user's environment to a degree).If a locked out feature is hit, a warning popup with as much details as possible should appear (e.g. with the address of a web request, or a filesystem path, or a shell command). There the user can be asked again if they want to trust the project, and a rejection would abort the requested feature.
This should probably also apply to running the project, either the act of running it itself (warn that the user tries to run an untrusted project), or apply the same locking mechanism to it as we would do to the editor environment.
The trust status is going to be stored in the editor's project cache with all other local project information. New projects created from a PM locally would be marked as trusted by default, while others would not be upon importing/opening via file association.
Note, that this proposal targets editor security specifically, and doesn't attempt to solve overall sandboxing which some desire for modding support and such. See https://github.com/godotengine/godot-proposals/issues/389 and https://github.com/godotengine/godot-proposals/issues/2689 for that.
If this enhancement will not be used often, can it be worked around with a few lines of script?
No. This is about security changes to the editor.
Is there a reason why this should be core and not an add-on in the asset library?
See above.