Bug 998227 (CVE-2013-4260)

Summary: CVE-2013-4260 ansible: predictible filename used for failed result in world writable directory
Product: [Other] Security Response Reporter: Michael S. <misc>
Component: vulnerabilityAssignee: Red Hat Product Security <security-response-team>
Status: CLOSED ERRATA QA Contact:
Severity: medium Docs Contact:
Priority: medium    
Version: unspecifiedCC: athmanem, kevin, maxim, security-response-team, tbielawa, vdanen, vkrizan
Target Milestone: ---Keywords: Security
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-09-05 22:48:26 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 999621, 999626    
Bug Blocks: 998712    
Attachments:
Description Flags
patch to use a different directory and check the permission after none

Description Michael S. 2013-08-18 12:17:28 UTC 
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 S. 2013-08-18 12:52:01 UTC 
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 16:07:00 UTC 
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 16:15:03 UTC 
Acknowledgements:

Red Hat would like to thank Michael Scherer for reporting this issue.
Comment 4 Michael S. 2013-08-20 20:27:17 UTC 
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 17:38:29 UTC 
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 17:59:14 UTC 
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 22:58:34 UTC 
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 23:03:37 UTC 
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.