Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop silently executing arbitrary code from the current directory by default #9514

Closed
the-dipsy opened this issue Feb 3, 2024 · 8 comments · May be fixed by #9545
Closed

Stop silently executing arbitrary code from the current directory by default #9514

the-dipsy opened this issue Feb 3, 2024 · 8 comments · May be fixed by #9545
Labels
C-bug Category: This is a bug R-duplicate Duplicated issue: please refer to the linked issue

Comments

@the-dipsy
Copy link

Summary

According to the docs about languages language configuration is read, among others, from the current projects .helix directory and allows specification of languages and language server commands. This can easily be exploited to make helix execute arbitrary code when started in an untrusted directory.

Vim disables reading project specific configurations by default and warns about enabling it for this very reason.

❗ PLEASE PATCH THIS AND DON'T USE HELIX IN ANY UNTRUSTED DIRECTORIES UNTIL THEN ❗

Reproduction Steps

  1. E. g. for a python project create a file .helix/languages.toml with the following content.
[language-server.evil]
command = "sh"
args = ["-c", "echo evil > evil.txt"]

[[language]]
name = "python"
language-servers = ["evil"]
  1. Start helix with hx some-file.py
  2. See a file evil.txt being automatically created
  3. Shiver in fear

Helix log

No response

Platform

Linux (probably all)

Terminal Emulator

all

Installation Method

releases page

Helix Version

23.10

@the-dipsy the-dipsy added the C-bug Category: This is a bug label Feb 3, 2024
@the-mikedavis
Copy link
Member

Duplicate of #2697

@the-mikedavis the-mikedavis marked this as a duplicate of #2697 Feb 3, 2024
@the-mikedavis the-mikedavis closed this as not planned Won't fix, can't repro, duplicate, stale Feb 3, 2024
@the-mikedavis the-mikedavis added the R-duplicate Duplicated issue: please refer to the linked issue label Feb 3, 2024
@the-dipsy
Copy link
Author

This is not a duplicate of #2697 because #2697 at least assumes that a language server is already installed. This issue even persists if the user has none installed. This vulnerability has already been mentioned in this comment but I can't find another issue about it. Is there at least an option to disable this behavior?

@the-mikedavis
Copy link
Member

See #7304, #1249 (comment).

Whether you're executing a language server or an arbitrary command doesn't really matter since some language servers can execute arbitrary scripts as part of compilation (for example a Rust build.rs script).

You can disable LSP altogether by setting editor.lsp.enable = false

@the-dipsy
Copy link
Author

That's true. My point is that a user who has no language server installed and therefore trusts that no code will be executed is still vulnerable to this. I for example deliberately refrain from installing any such software outside of virtualized or containerized environments on my machine but would still like to use a modern editor on it. Disabling LSPs all together will presumably fix this for now but I'm still convinced that this shouldn't be the default behavior.

@kirawi
Copy link
Member

kirawi commented Feb 4, 2024

That's the goal though as shown in #1249 (comment) which was linked above

@the-dipsy
Copy link
Author

the-dipsy commented Feb 4, 2024

I'd say, these are still different problems. I might have a language server that I trust not to execute user code and that I would like to enable for a specific workspace without being overwritten by the configuration files in it.

@lazytanuki
Copy link
Contributor

You can disable LSP altogether by setting editor.lsp.enable = false

This is not a fix for this specific issue. If in addition of shipping an evil .helix/languages.toml, an attacker can ship a .helix/config.toml file with editor.lsp.enable = true in it. Just tested it with my global editor.lsp.enable set to false, the attack still works.

Apart from this, I think this issue was closed a bit prematurely. I agree with @the-dipsy that it is a different problem.
I know that an attacker could run code using build scripts, but I don't think it justifies ignoring the issue of the local .helix folder altogether.
We could have a mechanism asking the user whether to load the local .helix configuration when there is one. Or, to make things simpler, we could have an option to disable the automatic loading of local .helix folders. Maybe it could show a warning when the directory is present and its loading disabled, so that the user can manually load it with a command or something like this. A bit like VSCode's workspace trust in a way.

It could also be nice to chose whether we want to load the local config for other reasons than security as well.

I think the right fix for this shouldn't be too hard to implement.

@pascalkuthe
Copy link
Member

the solution of both of these issues is the same: implement a workspace trust system similarly to the one used by vscode. Since the solution is the same only one issue is need and this is why we closed this one as duplicate

@helix-editor helix-editor locked and limited conversation to collaborators Feb 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C-bug Category: This is a bug R-duplicate Duplicated issue: please refer to the linked issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants