[OSSA 2013-023] Potential unsafe XML usage (CVE-2013-4179, CVE-2013-4202)

Bug #1190229 reported by Thierry Carrez on 2013-06-12
266
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Cinder
High
Thierry Carrez
Grizzly
High
Thierry Carrez
OpenStack Compute (nova)
High
Michael Still
Grizzly
High
Thierry Carrez
OpenStack Security Advisory
Medium
Thierry Carrez
neutron
Undecided
Unassigned

Bug Description

Grant Murphy (<email address hidden>) conducted an audit of OpenStack and reported the following potential XML related vulnerabilities. These may well not be exploitable, we need to doublecheck them.

===================================================
Issue #1 (CWE-776) - Widespread and inconsistent usage of XML libraries that are vulnerable to various XML resource exhaustion attack vectors. This was mostly addressed in bug 1100282 however I can still find instances of minidom.parseString used directly in contributors code:

nova/nova/virt/libvirt/driver.py
1640: dom = minidom.parseString(xml)
1657: dom = minidom.parseString(xml)

nova/nova/virt/xenapi/vm_utils.py
1501: rrd = minidom.parseString(xml)
1542: doc = minidom.parseString(xml)

nova/nova/api/openstack/compute/contrib/security_groups.py
542: root = minidom.parseString(req.body)

nova/nova/api/openstack/compute/contrib/security_group_default_rules.py
76: dom = minidom.parseString(string)

cinder/cinder/api/contrib/backups.py
85: dom = minidom.parseString(string)
104: dom = minidom.parseString(string)

cinder/cinder/api/contrib/volume_transfer.py
65: dom = minidom.parseString(string)
83: dom = minidom.parseString(string)

Furthermore there are inconsistencies in the fix between components. For example in Quantum they use a ProtectedXMLParser in quantum/quantum/wsgi.py for XMLDeserialization. The implementation is as follows:

class ProtectedXMLParser(etree.XMLParser):
    def __init__(self, *args, **kwargs):
        etree.XMLParser.__init__(self, *args, **kwargs)
        self._parser.StartDoctypeDeclHandler = self.start_doctype_decl

    def start_doctype_decl(self, name, sysid, pubid, internal):
        raise ValueError(_("Inline DTD forbidden"))

    def doctype(self, name, pubid, system):
        raise ValueError(_("Inline DTD forbidden"))

Although etree.XMLParser does not automatically expand external entities by default it is vulnerable to billion laughs and quadratic blowup according to the diffusedxml documentation. (I'm not sure this is still accurate though).

For consistency alone I would recommend shifting these libraries across the board:
 - https://pypi.python.org/pypi/defusedxml/
 - https://pypi.python.org/pypi/defusedexpat/

========================================================
Issue #2 (CWE-112)- Missing schema validation. There are several instances where XML data is processed from an external source that is not validated against an XML schema. This is a best practice that should be considered.

========================================================
Issue #3 (Probably not a bug)- HTTP parameters used directly in XML output. The output appears to use sax.xhtml_escape so in theory is safe. Could use a whitelist reject unexpected input.

The case I'm specifically talking about is the BucketHandler class in nova/nova/objectstore/s3server.py. This does:

  prefix = self.request.params.get("prefix", "")
  marker = self.request.params.get("marker", "")

And uses these values directly in render_xml (results sent to end user).
This one is probably harmless but thought I'd mention it anyway.

CVE References

Thierry Carrez (ttx) on 2013-06-12
Changed in ossa:
status: New → Incomplete
Thierry Carrez (ttx) wrote :

Analysis in Nova:

nova/nova/virt/libvirt/driver.py
What's getting parsed is XML from libvirt, not user-provided XML requests. So that's safe.

nova/nova/virt/xenapi/vm_utils.py
What's getting parsed is XML from XenServer, not user-provided XML requests, so that's safe.

nova/nova/api/openstack/compute/contrib/security_groups.py
While most calls use xmlutil.safe_minidom_parse_string (which is safe), there is one call in _extend_servers that still uses minidom.parseString(req.body) and therefore looks vulnerable.

nova/nova/api/openstack/compute/contrib/security_group_default_rules.py
Uses pure minidom in its XML deserializer, so probably vulnerable.

Changed in nova:
status: New → Confirmed
Thierry Carrez (ttx) wrote :

Quantum analysis:
My understanding is that million laughs/quadratic blowup needs DTD evaluation, and the ProtectedXMLParser protects against reading an included DTD, so that sounds safe.

I agree that more consistency across the board would be welcome though.

Changed in quantum:
status: New → Incomplete
Thierry Carrez (ttx) wrote :

Looks vulnerable to me (contributed backups API extension)

Changed in cinder:
status: New → Confirmed
Thierry Carrez (ttx) wrote :

Adding nova-core and cinder-core for help patching

Changed in ossa:
status: Incomplete → Confirmed
Thierry Carrez (ttx) on 2013-06-21
Changed in ossa:
importance: Undecided → Medium
Daniel Berrange (berrange) wrote :

Any patches should also include test case which validates that no code in GIT uses the vulnerable APIs, so we don't regress in this yet again.

Thierry Carrez (ttx) wrote :

We should push a minimal patch to fix the vulnerabilities first, that we can backport to stable branches.
Then for Havana and beyond we should push defusedxml parsers across the board.

Thierry Carrez (ttx) on 2013-07-03
Changed in neutron:
status: Incomplete → Invalid
Michael Still (mikalstill) wrote :

A patch for nova is attached.

Changed in nova:
importance: Undecided → High
assignee: nobody → Michael Still (mikalstill)
Thierry Carrez (ttx) wrote :

Could we have a patch for Cinder ?

Changed in nova:
status: Confirmed → In Progress
Changed in cinder:
assignee: nobody → John Griffith (john-griffith)
importance: Undecided → High
John Griffith (john-griffith) wrote :

cinder patch here

Thierry Carrez (ttx) wrote :

Looks good, could we have a cinder-core pre-approval here ? Patch is trivial so one should be enough

Thierry Carrez (ttx) wrote :

We need nova-core pre-approval on Mikal's patch too.

Pádraig Brady (p-draigbrady) wrote :

+2 On nova patch

Thierry Carrez (ttx) wrote :

Proposed impact description:

Title: Denial of Service using XML entities in some Nova and Cinder extensions
Reporter: Grant Murphy (Red Hat)
Products: Nova, Cinder
Affects: Grizzly

Description:
Grant Murphy from Red Hat reported that vulnerabilities in XML request parsers were not fully patched in OSSA 2013-004. By leveraging XML entity expansion in specific extensions, an unauthenticated attacker may still consume excessive resources on the Nova or Cinder API servers, resulting in a denial of service and potentially a crash. Only Nova setups making use of the security group extension in Grizzly are affected. Only Cinder setups making use of the backups or volume transfer API extension in Grizzly are affected.

Thierry Carrez (ttx) wrote :

The affected code was present in Grizzly but not in Folsom.

Changed in ossa:
assignee: nobody → Thierry Carrez (ttx)
status: Confirmed → Triaged
Jeremy Stanley (fungi) wrote :

Thierry's impact description in comment #14 looks sane to me.

Jeremy Stanley (fungi) wrote :

Sorry, Thierry's impact description in comment #13 looks sane to me.

Mike Perez (thingee) wrote :

+2 on the cinder patch.

Thierry Carrez (ttx) on 2013-07-17
Changed in ossa:
status: Triaged → In Progress
Thierry Carrez (ttx) wrote :

CVE requested, no answer from Kurt yet

Thierry Carrez (ttx) wrote :

Sent CVE request again

Thierry Carrez (ttx) on 2013-07-30
summary: - Potential unsafe XML usage
+ Potential unsafe XML usage (CVE-2013-4179)

Refreshed nova/havana patch

Thierry Carrez (ttx) wrote :

nova/grizzly patch backport

Thierry Carrez (ttx) wrote :

cinder/grizzly patch backport

Thierry Carrez (ttx) wrote :

cinder/master/gate-devstack-vm-full: PASS
cinder/grizzly/gate-devstack-vm-cinder: PASS

more tests tomorrow.

Thierry Carrez (ttx) wrote :

Nova tests fail with:

File "/opt/stack/new/nova/nova/api/openstack/compute/contrib/security_groups.py", line 33, in <module>
    from nova.openstack.common import xmlutils
ImportError: cannot import name xmlutils

Thierry Carrez (ttx) wrote :

missing a file in the grizzly backport. New version attached

Thierry Carrez (ttx) wrote :

nova/master/gate-devstack-vm-full: PASS
nova/grizzly/gate-devstack-vm-full: PASS

Thierry Carrez (ttx) wrote :

Version that doesn't fail pep8

Thierry Carrez (ttx) wrote :

Version that doesn't fail pep8

Thierry Carrez (ttx) wrote :

cinder/master/py27: PASS
cinder/master/pep8: PASS
nova/grizzly/py27: PASS
nova/grizzly/pep8: PASS
nova/master/py27: PASS
nova/master/pep8: PASS

Thierry Carrez (ttx) wrote :

Sent to downstream stakeholders
Proposed public disclosure date/time: Thursday, August 8, 1500UTC.

Changed in ossa:
status: In Progress → Fix Committed
Thierry Carrez (ttx) wrote :

From Kurt:

Please use CVE-2013-4179 for Denial of Service using XML entities in
Nova extensions

Please use CVE-2013-4202 for Denial of Service using XML entities in
Cinder extensions

summary: - Potential unsafe XML usage (CVE-2013-4179)
+ Potential unsafe XML usage (CVE-2013-4179, CVE-2013-4202)
Thierry Carrez (ttx) on 2013-08-08
information type: Private Security → Public Security

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

Changed in cinder:
assignee: John Griffith (john-griffith) → Thierry Carrez (ttx)
status: Confirmed → In Progress
Thierry Carrez (ttx) on 2013-08-08
summary: - Potential unsafe XML usage (CVE-2013-4179, CVE-2013-4202)
+ [OSSA 2013-023] Potential unsafe XML usage (CVE-2013-4179,
+ CVE-2013-4202)

Reviewed: https://review.openstack.org/40881
Committed: http://github.com/openstack/cinder/commit/4ad95dba4fccbbc0df923dea0dc9e5c3ac9f4cc2
Submitter: Jenkins
Branch: master

commit 4ad95dba4fccbbc0df923dea0dc9e5c3ac9f4cc2
Author: Thierry Carrez <email address hidden>
Date: Thu Aug 8 12:13:52 2013 +0200

    Use utils.safe_minidom_parse_string in extensions

    Use utils.safe_minidom_parse_string in extensions that were still
    using potentially-unsafe minidom.

    Fixes bug 1190229

    Change-Id: I43afb2e188bbea99ea30fe6cb2eb1aeedc4ddfd4

Changed in cinder:
status: In Progress → Fix Committed
Thierry Carrez (ttx) wrote :

OSSA 2013-023

Changed in nova:
status: In Progress → Fix Committed

Reviewed: https://review.openstack.org/40883
Committed: http://github.com/openstack/cinder/commit/2023eecc4b1a35daf42a64fa01967ed12c7d017b
Submitter: Jenkins
Branch: stable/grizzly

commit 2023eecc4b1a35daf42a64fa01967ed12c7d017b
Author: John Griffith <email address hidden>
Date: Thu Aug 8 12:10:30 2013 +0200

    Use utils.safe_minidom_parse_string in extensions

    Use utils.safe_minidom_parse_string in extensions that were still
    directly using potentially-unsafe minidom.

    Grizzly backport of John Griffith's master patch.

    Fixes bug 1190229

    Change-Id: I43afb2e188bbea99ea30fe6cb2eb1aeedc4ddfd4

Thierry Carrez (ttx) on 2013-08-08
Changed in ossa:
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2013-09-05
Changed in cinder:
milestone: none → havana-3
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2013-09-05
Changed in nova:
milestone: none → havana-3
status: Fix Committed → Fix Released

I will be out of the office starting 2013-09-05 and will not return until
2013-09-15.

I will be on my marriage leave from 9/5 to 9/15, for any urgent issue
please call me before 9/7.

For daily work, please ask my scrum master Zhu Zhu for help.
For glance issue, please ask glance SME Feilong Wang for help.
For defect report, there will be no report next week.

Thierry Carrez (ttx) on 2013-10-17
Changed in cinder:
milestone: havana-3 → 2013.2
Thierry Carrez (ttx) on 2013-10-17
Changed in nova:
milestone: havana-3 → 2013.2
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers