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

Do not use "sudo" with vim for security reasons #2151

Closed
thomasmerz opened this issue Jul 18, 2022 · 4 comments · Fixed by #2152
Closed

Do not use "sudo" with vim for security reasons #2151

thomasmerz opened this issue Jul 18, 2022 · 4 comments · Fixed by #2152

Comments

@thomasmerz
Copy link
Contributor

thomasmerz commented Jul 18, 2022

Use sudoedit for security reasons in

because sudo vim gives users a root shell by executing arbitrary shell commands.

PR will follow…

@gaelicWizard
Copy link
Contributor

I would definitely prefer to use the most correct tool for the job, but it doesn't seem like this is a vulnerability anywhere except a specially configured sudoers environment. If the user can already sudo then what happens after that is not a vulnerability.

I left a review on the PR asking for some logic to deal with not having a sudoedit binary on the system.

@thomasmerz
Copy link
Contributor Author

If a user already can do sudo everything, than I fully agree. But if a user is only allowed to sudo vim (any file) he/she/it can get a root-shell but normally this is not intended by allowing users editing all files. At home I'm fine that my user can sudo everything. But would never allow any non-root-granted person than me sudo vim.

@tbhaxor
Copy link
Contributor

tbhaxor commented Aug 2, 2022

I agree with @gaelicWizard, this is not a vulnerability but a misconfiguration I would say. We can have shell set to /bin/false, but it is mostly required on the servers.

We can also run vim with -Z flag, it will restrict executing shell commands.

@cornfeedhobo
Copy link
Member

Instead, I vote we remove any customizations and/or tools that require sudo, most especially, not in the general aliases.

This issue was closed.
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

Successfully merging a pull request may close this issue.

4 participants