eval being used in session.py

[gvb]

There's a FIXME comment saying eval is evil (which it is obviously) in
the file below:

/opt/stack/nova/nova/virt/xenapi/client/session.py

    def _unwrap_plugin_exceptions(self, func, *args, **kwargs):
        """Parse exception details."""
        try:
            return func(*args, **kwargs)
        except self.XenAPI.Failure as exc:
            LOG.debug("Got exception: %s", exc)
            if (len(exc.details) == 4 and
                exc.details[0] == 'XENAPI_PLUGIN_EXCEPTION' and
                    exc.details[2] == 'Failure'):
                params = None
                try:
                    # FIXME(comstud): eval is evil.
                    params = eval(exc.details[3]) <--- here
                except Exception:
                    raise exc
                raise self.XenAPI.Failure(params)
            else:
                raise
        except xmlrpclib.ProtocolError as exc:
            LOG.debug("Got exception: %s", exc)
            raise

This should indeed be fixed as it looks that an arbitrary plugin can now
potentially achieve arbitrary code execution by throwing specifically
prepared XenAPI Failure Exceptions.

This code has been here from the beginning according to the git logs at
http://git.openstack.org/cgit/openstack/nova/log/nova/virt/xenapi/client/session.py
so the FIXME coomment has existed since at least 2013-11-22.

[Tristan Cacqueray (tristan-cacqueray)]

Since this report concerns a possible security risk, an incomplete security advisory task has been added while the core security reviewers for the affected project or projects confirm the bug and discuss the scope of any vulnerability along with potential solutions.

[Tristan Cacqueray (tristan-cacqueray)]

From a security point of view, this bug report does not seems to describe a vulnerability: an arbitrary plugin goal is to achieve arbitrary code execution... and in order to reach this eval, the plugin must be installed first.

However this code might deserve some strengthening... I suggest to remove the OSSA task and open this report.

Let me know if that works for you.

[gvb (gvb-2)]

Works for me.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Change abandoned by Abhilash Goyal (<email address hidden>) on branch: master
Review: https://review.openstack.org/276048
Reason: other patch available.

[Michael Glaser (mikeg451)]

Abhilash,
Your solution looks good and is still an active commit, so why did you set the assignee to nobody?

[Markus Zoeller (markus_z) (mzoeller)]

Change https://review.openstack.org/#/c/275513/ is fixing this and looks good.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/275513
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=9955d1685d3a5a4357f405b442abaa43f60cd6d0
Submitter: Jenkins
Branch: master

commit 9955d1685d3a5a4357f405b442abaa43f60cd6d0
Author: abhilash-goyal <email address hidden>
Date: Tue Feb 9 16:25:13 2016 +0530

    Replace use of eval with ast.literal_eval

    literal_eval supports a limited subset of Python, and is therefore
    safer than eval.

    Close-bug: 1414529
    Change-Id: Ib6145408360fa57cccc8d77c590a203e5088b193

[Sean Dague (sdague)]

There are no currently open reviews on this bug, changing
the status back to the previous state and unassigning. If
there are active reviews related to this bug, please include
links in comments.