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

Potential security issue #154

Closed
mkcms opened this issue Nov 17, 2018 · 11 comments
Closed

Potential security issue #154

mkcms opened this issue Nov 17, 2018 · 11 comments

Comments

@mkcms
Copy link
Collaborator

mkcms commented Nov 17, 2018

The function eglot--format-markup calls a function chosen by the server, to enable a major mode which will fontify a string. However, it doesn't check if this function is safe to call, or if the function would enable some major mode at all.
This makes it a potential security issue.

For example, evaluating the following code will make Emacs unkillable (except by a signal):

(eglot--format-markup '(:value "" :language "emacs-lock"))

These will trigger a nonlocal exit:

(eglot--format-markup '(:value "1" :language "read-only"))
(eglot--format-markup '(:value "1" :language "edebug"))
@joaotavora
Copy link
Owner

This makes it a potential security issue.

...in the (reasonably rare) case that you're connecting to server to severs running non-locally, otherwise it's an attack vector totally contained inside another attack surface.

@mkcms
Copy link
Collaborator Author

mkcms commented Nov 19, 2018

...in the (reasonably rare) case that you're connecting to server to severs running non-locally, otherwise it's an attack vector totally contained inside another attack surface.

We do make it possible nevertheless, without even any warning.
I don't think anybody would ever expect a server to do anything other than providing information to the editor.

Also, it is a security issue in other situations. Consider the scenario where you install pyls from PyPI and run it as another user from within emacs. After one update to pyls (maybe done automatically), an attacker can literally do something on another user account.

I strongly support removing this dangerous behavior.

@joaotavora
Copy link
Owner

Also, it is a security issue in other situations. Consider the scenario where you install pyls from PyPI and run it as another user from within emacs.

You mean installing pyls as the root user surely. That's the only other plausible scenario. If so, and if pyls is compromised, then you've already lost long before it tries to trick emacs into running some maleovolent elisp function.

I strongly support removing this dangerous behavior.

I agree. I was just helping you measure the extent of the "danger"

@mkcms
Copy link
Collaborator Author

mkcms commented Nov 19, 2018

You mean installing pyls as the root user surely

Why? You can run these programs as an unprivileged user, run them with different cgroups, or even in a container which shares some part of a filesystem.

@joaotavora
Copy link
Owner

You mean installing pyls as the root user surely

Why?

I meant to ask this: in what situations do you see yourself installing pyls a user X and running it as user Y? The only plausible situation where I can imagine this is if X is root and Y is a normal, unpriveliged, user. Do you see any more? If so what are X and Y?

@mkcms
Copy link
Collaborator Author

mkcms commented Nov 19, 2018

I meant to ask this: in what situations do you see yourself installing pyls a user X and running it as user Y?

I can imagine myself doing this:

  1. Install pyls as non-root user X in $HOME
  2. Start emacs as user X
  3. Evaluate (add-hook 'python-mode-hook #'eglot-ensure)
  4. Find the file /etc/unimportant.py (still, as user X)
  5. Find the file /sudo::/etc/important.py because completion failed me when I wanted /sudo::/etc/important.pie (which can happen because when you hit TAB during typing, emacs displays a password prompt, and you can instinctively press RET to hide Tramp's "connected" message)
  6. Because I am visiting a buffer with /sudo:: I can now do (shell-command-to-string "cat /etc/shadow")

@joaotavora
Copy link
Owner

joaotavora commented Nov 19, 2018 via email

@mkcms
Copy link
Collaborator Author

mkcms commented Nov 19, 2018

Indeed, given this attack vector, why would anyone target pyls/eglot and not just some elisp library???

Maybe because even if there is an Emacs library insert-gpg-key-in-random-buffer-mode which is used by advanced users, an attacker still needs a way to call it from outside Emacs :-)

@joaotavora
Copy link
Owner

Maybe because even if there is an Emacs library insert-gpg-key-in-random-buffer-mode which is used by advanced users, an attacker still needs a way to call it from outside Emacs :-)

He'll probably find it's much easier to use emacsclient for that

@mkcms
Copy link
Collaborator Author

mkcms commented Nov 19, 2018

He'll probably find it's much easier to use emacsclient for that

Interesting, although AFAICS it's not possible for a non-root user to access other's sockets, I tried running a server and then emacsclient as another user.

emacsclient: Invalid socket owner
emacsclient . -s /tmp/emacs1000/server
emacsclient: can't stat /tmp/emacs1000/server: Permission denied

@joaotavora
Copy link
Owner

But I thought your use case is to run code as a non-priviledged user and then stealthily wait for the user to use tramp and /sudo::, right? Emacsclient is fine for that

bhankas pushed a commit to bhankas/emacs that referenced this issue Sep 18, 2022
… doc

Previously, a server could mistankely or maliciously call *-mode
functions by in the response to a completion or hover request,
specifically in the :documentation field of the response.

Although there are plenty of similar avenues of attack in Emacs, it's
probably a good idea not to let LSP servers decide which functions to
call in an Emacs session running Eglot.

* eglot.el (eglot--format-markup): Call major-mode to fontify
buffer, not some dynamically constructed function name.
(eglot-completion-at-point): Ensure eglot--format-markup runs in
source buffer.
bhankas pushed a commit to bhankas/emacs that referenced this issue Sep 19, 2022
… doc

Previously, a server could mistankely or maliciously call *-mode
functions by in the response to a completion or hover request,
specifically in the :documentation field of the response.

Although there are plenty of similar avenues of attack in Emacs, it's
probably a good idea not to let LSP servers decide which functions to
call in an Emacs session running Eglot.

* eglot.el (eglot--format-markup): Call major-mode to fontify
buffer, not some dynamically constructed function name.
(eglot-completion-at-point): Ensure eglot--format-markup runs in
source buffer.
bhankas pushed a commit to bhankas/emacs that referenced this issue Sep 19, 2022
Previously, a server could mistankely or maliciously call *-mode
functions by in the response to a completion or hover request,
specifically in the :documentation field of the response.

Although there are plenty of similar avenues of attack in Emacs, it's
probably a good idea not to let LSP servers decide which functions to
call in an Emacs session running Eglot.

* eglot.el (eglot--format-markup): Call major-mode to fontify
buffer, not some dynamically constructed function name.
(eglot-completion-at-point): Ensure eglot--format-markup runs in
source buffer.

#154: joaotavora/eglot#154
jollaitbot pushed a commit to sailfishos-mirror/emacs that referenced this issue Oct 12, 2022
Previously, a server could mistankely or maliciously call *-mode
functions by in the response to a completion or hover request,
specifically in the :documentation field of the response.

Although there are plenty of similar avenues of attack in Emacs, it's
probably a good idea not to let LSP servers decide which functions to
call in an Emacs session running Eglot.

* eglot.el (eglot--format-markup): Call major-mode to fontify
buffer, not some dynamically constructed function name.
(eglot-completion-at-point): Ensure eglot--format-markup runs in
source buffer.

GitHub-reference: fix joaotavora/eglot#154
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants