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
Malicious scripts in untrusted directories are executed #122
Comments
|
Hi @saleemrashid this is true. But if I understand this correctly, apart from the automation it is no more or less secure than any virutalenv activation script (Please feel free to correct me if I am wrong here). If there was no automation the user would have activated the corresponding virtualenv manually. A couple of checks are made to make sure the correct virtualenv target is not tampered with https://github.com/MichaelAquilina/zsh-autoswitch-virtualenv/blob/master/autoswitch_virtualenv.plugin.zsh#L140-L147 We could probably look into making sure the virtualenv files are not writable except by the user too (using chown) |
|
@MichaelAquilina The point here is that the user isn't expecting there to be a virtualenv, so they would not have activated it themselves. They are cloning a random Git repository, that they don't necessarily trust, and by simply running This is a huge issue, unless you believe that you will never This is why, for example, direnv has a security mechanism that requires you to whitelist directories (e.g. run |
|
I think your threat model is incredibly unusual:
|
Yep that makes it clearer to me. I now realise how the contents of
That's an interesting approach which we can probably streamline without the user. Preventing unallowed chars is probably a huge improvement too (
I appreciate you are trying to be helpful, but there's no need to take a dramatic tone here. Help me understand the issue instead :)
If you are sharing a machine, then this check prevents anyone from making you execute a malicious virtualenv by allowing them to over write the target (which could be made malicious).
I'm curious to see how you would even detect this? A file you downloaded on your system is just a file in the end. Next time, I would appreciate you consider responsible disclosure and email me directly if you think there is a threat though. |
|
@saleemrashid please take a look at the latest release and see what you think. I have a few other ideas I will implement but the issue you specifically posted should be resolved. |
Sorry for the lack of clarity. For completeness, it is not necessary to use the exact number of path traversals because
I understand the concept, but let us imagine that your
This kind of vulnerability is common to all utilities of this nature, so I am confident that anyone who wants to exploit it would spot it without my help. I don't use this plugin, but someone I follow on GitHub happened to star it so I figured I would take a look and see if it had this vulnerability. Had the situation been different, e.g. you had implemented a security mechanism and I discovered a bypass for it, then I would have reached out to you privately. |
|
I still think you should consider implementing white-listing. Even if your validation for |
Agreed there is more work to be done. Just wanted to fix the main issue for now in order to get it out asap. Thanks for posting this issue! |
git clone https://github.com/saleemrashid/evil-zsh-autoswitch-virtualenv.git cd evil-zsh-autoswitch-virtualenv cat evil.txtThe script
evil_virtualenv/bin/activatewill be sourced without any user interaction, which will:id; ls ~to a file calledevil.txt. Obviously this would be more malicious in practiceThe text was updated successfully, but these errors were encountered: