Since version 1.2 of ansible, failed run ( due to connexion errors, or config error ) are listed into /var/tmp/ansible/$script_name.yml , with $script_name being the script name used ( or rather the playbook, in ansible linguo ) There is no verification on the file or directory here, and /var/tmp is world writable. Worst, due to it using a subdirectory under /var/tmp, some symlink protection may not apply ( not tested ). For example, if i create a directory /var/tmp/ansible with owner misc:users and a symlink to a file of joe, the kernel would permit to follow since the symlink and owner of the directory match. This permit to erase file content among others. I am not sure what kind of specific attack could be made by injecting ip and hostname in a specific file, but I am sure this exist. Code is on https://github.com/ansible/ansible/blob/devel/lib/ansible/playbook/__init__.py#L480 Upstream was not notified yet AFAIK. I do have a patch almost ready that do : - verify the permission/owner of directory - create a unique directory derived from username ( so predictable ) with proper permission if doesn't exist I just need to review and test. The current code do cope with lack of permission on the directory so even if someone create a directory in advance, this will be handled "gracefully" ( I think a message would be better )
Created attachment 787776 [details] patch to use a different directory and check the permission after Here is a patch that should fix the problem, I quickly tested and seems to be resilient enough. I may have forgot something about symlinks however, so a review would be welcome.
Why not use tempfile.mkdtemp? http://docs.python.org/2/library/tempfile.html#tempfile.mkdtemp So something like this: #!/usr/bin/env python import tempfile import os f = tempfile.mkdtemp(prefix='foo', dir='/tmp') try: os.rename(f, '/tmp/foo') except OSError: print 'Unable to rename directory!' os.rmdir(f) Probably better exception handling to see if the /tmp/foo directory is valid and owned by that user first, but for the actual creation, mkdtemp() will do so securely and os.rename will do so atomically.
Acknowledgements: Red Hat would like to thank Michael Scherer for reporting this issue.
I can hardly see how/where there is a need to create the directory in a atomic fashion in the first place, and since check (if the directory /tmp/foo exist and is suitable ) and rename would not be atomic, then we would have a race condition. If someone create it between the time I check and the time I create the dir, the makedir will fail, and so directory is not used. And the owner wil be incorrect, since user cannot make chown to give file ( unless people have been playing with CAP_CHOWN but i will count that as "unlikely" ) Since the code is supposed to be able to fail, it is better to use this possibility in case of problem. But we will see the opinion of upstream.
Upstream release with fixes announced: https://groups.google.com/forum/#!topic/ansible-project/UVDYW0HGcNg Can we unembargo and setup updates bugs links for the updates? Thanks.
Created ansible tracking bugs for this issue: Affects: fedora-all [bug 999621] Affects: epel-6 [bug 999626]
ansible-1.2.3-2.fc18 has been pushed to the Fedora 18 stable repository. If problems still persist, please make note of it in this bug report.
ansible-1.2.3-2.fc19 has been pushed to the Fedora 19 stable repository. If problems still persist, please make note of it in this bug report.