suds client subject to cache poisoning by local attacker

Bug #1341954 reported by Grant Murphy
262
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
Fix Released
Undecided
Vipin Balachandran
Havana
Fix Released
Undecided
Unassigned
Icehouse
Fix Released
Undecided
Vipin Balachandran
OpenStack Compute (nova)
Fix Released
Medium
Davanum Srinivas (DIMS)
OpenStack Security Advisory
Won't Fix
Medium
Unassigned
OpenStack Security Notes
Fix Released
High
Tim Kelsey
gantt
New
Undecided
Unassigned
oslo.vmware
Fix Released
Undecided
Davanum Srinivas (DIMS)

Bug Description

The suds project appears to be largely unmaintained upstream. The default cache implementation stores pickled objects to a predictable path in /tmp. This can be used by a local attacker to redirect SOAP requests via symlinks or run a privilege escalation / code execution attack via a pickle exploit.

cinder/requirements.txt:suds>=0.4
gantt/requirements.txt:suds>=0.4
nova/requirements.txt:suds>=0.4
oslo.vmware/requirements.txt:suds>=0.4

The details are available here -
https://bugzilla.redhat.com/show_bug.cgi?id=978696
(CVE-2013-2217)

Although this is an unlikely attack vector steps should be taken to prevent this behaviour. Potential ways to fix this are by explicitly setting the cache location to a directory created via tempfile.mkdtemp(), disabling cache client.set_options(cache=None), or using a custom cache implementation that doesn't load / store pickled objects from an insecure location.

CVE References

Revision history for this message
Thierry Carrez (ttx) wrote :

This sounds OSSA worthy to me...

Thierry Carrez (ttx)
Changed in ossa:
status: New → Incomplete
Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

@ttx, I agree, this can lead to local code execution by local user in services context.

Changed in ossa:
status: Incomplete → Confirmed
importance: Undecided → Medium
Revision history for this message
Grant Murphy (gmurphy) wrote :

I think we can reuse the existing CVE for this issue and issue an advisory. I've created the draft impact description below:

Title: oslo.vmware uses a version of the suds soap client with known vulnerabilities.
Reporter: Grant Murphy (Red Hat)
Products: oslo.vmware, Nova, Cinder
Versions: from 2013.2 to 2013.2.3, and 2014.1 versions up to 2014.1.1

Description:
Grant Murphy from Red Hat found that oslo.vmware uses a vulnerable dependency.
The suds soap client cache stores pickled objects at a predictable path in /tmp.
A local attacker could pre-emptively create poisoned cache entries to execute
arbitrary code via pickle deserialization. The oslo.vmware code can be found
in the Nova and Cinder projects.

References:
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-2217

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

@Grant, Small nits:

Products: Nova, Cinder (we don't support oslo.vmware iirc)

Versions: up to 2013.2.3, 2014.1 versions up to 2014.1.1 (grizzly at least looks affected too, see:
$ git show grizzly-2:tools/pip-requires | grep suds
suds==0.4
)

I would merge the two first sentences -> "oslo.vmware uses a vulnerable version of the suds soap client that cache stores pickled objects at a predictable path in /tmp."

"A local attacker" -> "A local attacker with shell access"

"The oslo.vmware code can be found in the Nova and Cinder projects." -> "All Nova and Cinder setups are affected."

Revision history for this message
Grant Murphy (gmurphy) wrote :

Attaching proposed fix for oslo.vmware. This is probably not the cleanest solution. It uses mkdtemp to create a secure temporary directory to store cached objects. This directory will be cleaned up when object is out of scope. It may be a better to use a contextmanager for this however I was unsure of the typical usage. Another alternative would be to register a cleanup function via atexit.

How do you guys think we should patch this?

Revision history for this message
Grant Murphy (gmurphy) wrote :

 @Tristan - Thanks for the feedback. I think I was looking at cinder which tags which start at 2013.2. You're right though. Nova is affected from 2013.1. How's this one sound:

Title: oslo.vmware uses a version of the suds soap client with known vulnerabilities.
Reporter: Grant Murphy (Red Hat)
Products: Nova, Cinder
Versions: from 2013.1 to 2013.2.3, and 2014.1 versions up to 2014.1.1

Description:
Grant Murphy from Red Hat found that oslo.vmware uses a vulnerable version of the suds soap client that stores pickled objects at a predictable path in /tmp for caching purposes. A local attacker with shell access could pre-emptively create poisoned cache entries to execute arbitrary code when cached objects are deserialized. All Nova and Cinder setups are affected.

References:
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-2217

Revision history for this message
Tristan Cacqueray (tristan-cacqueray) wrote :

@Grant, the patch is looking good to me, though using __del__ for tmp clean can go wrong (as you don't control destruction order), maybe we could put the cleaning code into a function and call it in __del__ and as an atexit handler just to be on the safe side ?

Also, here (Ubuntu 13.10) suds does not install correctly with py33: http://paste.openstack.org/show/OebG2GJnDRTgm9JpzdMM/

+1 on impact description in comment #6.

Revision history for this message
Grant Murphy (gmurphy) wrote :

@Tristan I don't like the __del__ much either. I might rewrite it to do the atexit cleanup and define the cache directory at the oslo/vmware/api.py level and pass it in as a configuration parameter.

The incompatibility with Python 3.3. is also concerning with this library. Even the more up to date fork [1] has problems when you try to import the module in Python 3.3. Unfortunately there doesn't seem to be many other contenders in this space either [2].

[1] https://bitbucket.org/jurko/suds/overview
[2] http://stackoverflow.com/questions/206154/what-soap-client-libraries-exist-for-python-and-where-is-the-documentation-for

Revision history for this message
Thierry Carrez (ttx) wrote :

Maybe shorter title: "oslo.vmware uses a vulnerable version of the suds soap client"

Changed in ossa:
status: Confirmed → Triaged
Revision history for this message
Grant Murphy (gmurphy) wrote :

This patch removes the use of __del__() in favour of a call out to atexit. The caching can now also explicitly be disabled.

Revision history for this message
Grant Murphy (gmurphy) wrote :
Revision history for this message
Thierry Carrez (ttx) wrote :

@oslo-core please review proposed patch (or point to oslo.vmware people we should invite to this bug)

Revision history for this message
Ben Nemec (bnemec) wrote :

I'm a little confused here - from what I see suds defaults to no caching (https://jortel.fedorapeople.org/suds/doc/suds.options.Options-class.html), so since oslo.vmware wasn't providing any cache setting, suds wouldn't have been writing files to disk, would it? This appears to only be a problem if you use the FileCache without providing a location, which we weren't doing in the first place.

That said, the change generally looks fine to me. A few minor comments, but nothing that would change how it functions:

* If the directory cleanup fails, I would prefer to log the exception at the same level as the message. It's not something that should happen, so we want to provide information on why it did. Requiring debug logging to figure out problems in OpenStack is something we really need to get away from.

* self.cache should be either self._cache or just cache, since it doesn't appear to need to be a member variable.

* The tests should use the unit test assert methods - self.assertTrue/False(os.path.exists...)

Revision history for this message
Jeremy Stanley (fungi) wrote :

If we can confirm that we're not using suds in a vulnerable manner in either of Havana or Icehouse, then we shouldn't need to publish a security advisory for this and can work it in the open as a normal bug.

Revision history for this message
Grant Murphy (gmurphy) wrote :

I think the documentation does not actually represent what is happening in the code.

[gm@lappy suds_test]$ virtualenv venv
New python executable in venv/bin/python
Installing setuptools, pip...done.
[gm@lappy suds_test]$ source venv/bin/activate
(venv)[gm@lappy suds_test]$ pip install suds==0.4
Downloading/unpacking suds==0.4
  Downloading suds-0.4.tar.gz (104kB): 104kB downloaded
  Running setup.py (path:/home/gm/suds_test/venv/build/suds/setup.py) egg_info for package suds

Installing collected packages: suds
  Running setup.py install for suds
    /home/gm/suds_test/venv/bin/python -O /tmp/tmpqRpfM4.py
    removing /tmp/tmpqRpfM4.py

Successfully installed suds
Cleaning up...
(venv)[gm@lappy suds_test]$ python
Python 2.7.5 (default, Jun 25 2014, 10:19:55)
[GCC 4.8.2 20131212 (Red Hat 4.8.2-7)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import suds
>>> wsdl = "http://wsf.cdyne.com/WeatherWS/Weather.asmx?WSDL"
>>> c = suds.client.Client(wsdl)
>>> c.options.cache.location
'/tmp/suds'
>>> c.options.cache.__class__
<class suds.cache.ObjectCache at 0x7fe90c9aba78>

See also -
https://github.com/infirmary/suds/blob/master/suds/client.py#L109
https://github.com/infirmary/suds/blob/master/suds/cache.py#L314

Revision history for this message
Jeremy Stanley (fungi) wrote :

Agreed--I tried the same and also confirmed that the /tmp/suds directory is indeed created and used to store pickled data. It does indeed appear that suds has caching on by default after all.

Revision history for this message
Vipin Balachandran (vbala) wrote :

I tried with the latest version of oslo.vmware and it created the directory "tmpoKkE2U" with files of the format suds-*-document.px. There was no /tmp/suds directory. The same was observed while running the vmware cinder driver.

Revision history for this message
Grant Murphy (gmurphy) wrote :

@vbala

I think the dependency might be getting picked up via your vendors packaging system it could potentially be patched against this flaw. For example this package is patched: python-suds_0.4.1-2ubuntu1.1_all.deb for ubuntu.

Revision history for this message
Ben Nemec (bnemec) wrote :

Ah, they default the Client class differently from the Options class. In that case I agree there would be an issue in any unpatched versions of suds, so we should definitely fix this.

Revision history for this message
Thierry Carrez (ttx) wrote :

So... this could easily be considered a vulnerability in suds rather than in OpenStack, in which case it would not be OSSA territory. In particular, if most distros consider it a vulnerability in suds, we can workaround the issue in master (and even backport the fix) to care for vulnerable suds, but would not issue an OSSA about it. We could issue an OSSN about it though.

Revision history for this message
Thierry Carrez (ttx) wrote :

Also it would be well-known, so this could be made public.

Revision history for this message
Grant Murphy (gmurphy) wrote :

Sure. That makes sense. I think we need to look at reviving upstream or finding an alternative too. There are python 3 incompatibilities IIUC.

I'll cancel the OSSA task, add OSSN task bug, and switch to public if nobody screams about it soon.

Revision history for this message
Ben Nemec (bnemec) wrote :

Sounds fine to me.

Grant Murphy (gmurphy)
Changed in ossa:
status: Triaged → Won't Fix
information type: Private Security → Public Security
Changed in cinder:
assignee: nobody → Vipin Balachandran (vbala)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

Fix proposed to branch: master
Review: https://review.openstack.org/116270

Changed in cinder:
status: New → In Progress
Revision history for this message
Davanum Srinivas (DIMS) (dims-v) wrote :

Fixed in oslo.vmware - Ieec9d99aa674adf5cbc9be924fef3856cf4e5d66

Changed in oslo.vmware:
assignee: nobody → Davanum Srinivas (DIMS) (dims-v)
status: New → Fix Released
Tracy Jones (tjones-i)
tags: added: security
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (master)

Reviewed: https://review.openstack.org/116270
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=6a41fe9c5c98a14a355fa81b41aae2c4b0ce0a7b
Submitter: Jenkins
Branch: master

commit 6a41fe9c5c98a14a355fa81b41aae2c4b0ce0a7b
Author: Vipin Balachandran <email address hidden>
Date: Fri Aug 22 10:19:29 2014 +0530

    VMware: Disable suds caching

    The default cache implementation in suds store pickled objects in a
    predictable path in /tmp which could lead to attacks. This patch
    turns off suds caching to address this security issue.

    Change-Id: I7daaa25a0677004e03896298e9c3026d5c33c6ac
    Closes-Bug: #1341954

Changed in cinder:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/icehouse)

Fix proposed to branch: stable/icehouse
Review: https://review.openstack.org/118560

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (stable/havana)

Fix proposed to branch: stable/havana
Review: https://review.openstack.org/118565

Thierry Carrez (ttx)
Changed in cinder:
milestone: none → juno-3
status: Fix Committed → Fix Released
Revision history for this message
Davanum Srinivas (DIMS) (dims-v) wrote :

Here's a oslo.vmware review that adds back a memory based cache - https://review.openstack.org/#/c/116297/

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/icehouse)

Reviewed: https://review.openstack.org/118560
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=03573a02a3235db2bf65d48aceb92a6e81850dd9
Submitter: Jenkins
Branch: stable/icehouse

commit 03573a02a3235db2bf65d48aceb92a6e81850dd9
Author: Vipin Balachandran <email address hidden>
Date: Fri Aug 22 10:19:29 2014 +0530

    VMware: Disable suds caching

    The default cache implementation in suds store pickled objects in a
    predictable path in /tmp which could lead to attacks. This patch
    turns off suds caching to address this security issue.

    Change-Id: I7daaa25a0677004e03896298e9c3026d5c33c6ac
    Closes-Bug: #1341954
    (cherry picked from commit 6a41fe9c5c98a14a355fa81b41aae2c4b0ce0a7b)

tags: added: in-stable-icehouse
Revision history for this message
Davanum Srinivas (DIMS) (dims-v) wrote :

This has been fixed in oslo.vmware which Nova now depends on, we just need a fresh release of oslo.vmware to fix this in Nova

Changed in nova:
importance: Undecided → Medium
status: New → Confirmed
Changed in nova:
assignee: nobody → Davanum Srinivas (DIMS) (dims-v)
Changed in oslo.vmware:
milestone: none → juno-rc1
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to cinder (stable/havana)

Reviewed: https://review.openstack.org/118565
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=3c3530db2445eca9be583715ff689bd746e6d387
Submitter: Jenkins
Branch: stable/havana

commit 3c3530db2445eca9be583715ff689bd746e6d387
Author: Vipin Balachandran <email address hidden>
Date: Fri Aug 22 10:19:29 2014 +0530

    VMware: Disable suds caching

    The default cache implementation in suds store pickled objects in a
    predictable path in /tmp which could lead to attacks. This patch
    turns off suds caching to address this security issue.

    Conflicts:
            cinder/volume/drivers/vmware/pbm.py

    Change-Id: I7daaa25a0677004e03896298e9c3026d5c33c6ac
    Closes-Bug: #1341954
    (cherry picked from commit 6a41fe9c5c98a14a355fa81b41aae2c4b0ce0a7b)

tags: added: in-stable-havana
Revision history for this message
Sean Dague (sdague) wrote :

This should be addressed by the oslo.vmware bump in requirements

Changed in nova:
milestone: none → juno-rc1
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to nova (master)

Fix proposed to branch: master
Review: https://review.openstack.org/121614

Changed in nova:
status: Confirmed → In Progress
satyadev svn (svnsatya)
tags: added: vmware
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on nova (master)

Change abandoned by Davanum Srinivas (dims) (<email address hidden>) on branch: master
Review: https://review.openstack.org/121614

Changed in nova:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in nova:
status: Fix Committed → Fix Released
Tim Kelsey (tim-kelsey)
Changed in ossn:
assignee: nobody → Tim Kelsey (tim-kelsey)
Tim Kelsey (tim-kelsey)
Changed in ossn:
status: New → In Progress
Thierry Carrez (ttx)
Changed in nova:
milestone: juno-rc1 → 2014.2
Thierry Carrez (ttx)
Changed in cinder:
milestone: juno-3 → 2014.2
Changed in ossn:
importance: Undecided → High
Revision history for this message
Tom Fifield (fifieldt) wrote :

Tim, are you still working on this?

Revision history for this message
Nathan Kinder (nkinder) wrote :

This has been published as OSSN-0038 to the openstack and openstack-dev mailing lists as well as the wiki:

  https://wiki.openstack.org/wiki/OSSN/OSSN-0038

Changed in ossn:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public Security information  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.