This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 998227 - (CVE-2013-4260) CVE-2013-4260 ansible: predictible filename used for failed result in world writable directory
CVE-2013-4260 ansible: predictible filename used for failed result in world w...
Status: CLOSED ERRATA
Product: Security Response
Classification: Other
Component: vulnerability (Show other bugs)
unspecified
Unspecified Unspecified
medium Severity medium
: ---
: ---
Assigned To: Red Hat Product Security
impact=moderate,public=20130821,repor...
: Security
Depends On: 999621 999626
Blocks: 998712
  Show dependency treegraph
 
Reported: 2013-08-18 08:17 EDT by Michael Scherer
Modified: 2015-08-19 05:21 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-09-05 18:48:26 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
patch to use a different directory and check the permission after (2.16 KB, patch)
2013-08-18 08:52 EDT, Michael Scherer
no flags Details | Diff

  None (edit)
Description Michael Scherer 2013-08-18 08:17:28 EDT
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 )
Comment 1 Michael Scherer 2013-08-18 08:52:01 EDT
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.
Comment 2 Vincent Danen 2013-08-20 12:07:00 EDT
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.
Comment 3 Vincent Danen 2013-08-20 12:15:03 EDT
Acknowledgements:

Red Hat would like to thank Michael Scherer for reporting this issue.
Comment 4 Michael Scherer 2013-08-20 16:27:17 EDT
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.
Comment 5 Kevin Fenzi 2013-08-21 13:38:29 EDT
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.
Comment 6 Vincent Danen 2013-08-21 13:59:14 EDT
Created ansible tracking bugs for this issue:

Affects: fedora-all [bug 999621]
Affects: epel-6 [bug 999626]
Comment 7 Fedora Update System 2013-08-30 18:58:34 EDT
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.
Comment 8 Fedora Update System 2013-08-30 19:03:37 EDT
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.

Note You need to log in before you can comment on or make changes to this bug.