Several command injection vulnerabilities in guestagent/pkg

[Travis McPeak]

At several places in the file guestagent/pkg.py, there are shell injection vulnerabilities:

https://github.com/openstack/trove/blob/master/trove/guestagent/pkg.py#L209

In this line, the cmd_list is being built parameterized, but then it is just combined into one big string and called directly on a shell through the command getstatusoutput, which does a popen. If package name is set maliciously, the command will execute arbitrary code with the privilege of the trove process.

The same is true on this line, https://github.com/openstack/trove/blob/master/trove/guestagent/pkg.py#L258 , where a package named something like "abc; rm -rf /etc" will cause all files in /etc which Trove has permissions for, to be deleted.

Again, on this line: https://github.com/openstack/trove/blob/master/trove/guestagent/pkg.py#L371 , a malicious package name will cause arbitrary code injection with the privileges of the Trove process.

I'm not nearly familiar enough with the Trove code and uses to know all the ways that package names for this code can be set, but these commands should be parameterized.

Finally, os.popen is a deprecated function. The subprocess module should be used instead.

[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.

Does user have control over package name ?

[Travis McPeak (travis-mcpeak)]

In particular, since this code seems designed to run on Trove guests, I'm wondering if this could be used to execute commands somehow on a different guest. I'm not familiar enough with the architecture to know, but running malicious package names causing arbitrary code execution on a guest seems like an interesting vector.

[Travis McPeak (travis-mcpeak)]

^ Above statement wasn't clear, should have been more like "running these commands for malicious package names".

[Grant Murphy (gmurphy)]

The commands is a deprecated and should not be used anyway. There are a number of cases there where they are using pexpect too that probably need to be examined.

FWIW both yum and dpkg have pretty good python bindings that might be a safer way of implementing this kind of thing.

[Grant Murphy (gmurphy)]

I dug into this a bit today. I think the packages exist within the database and AFAICT don't seem to be something that be messed with by a malicious tenant. I am leaning towards a C1 or even D classification [1] for this bug. I suggest we fix this as security hardening and open the report up next Monday unless people find a practical exploit case before then.

1. Our vulnerability taxonomy is available here: https://wiki.openstack.org/wiki/Vulnerability_Management

[Nikhil Manchanda (slicknik)]

As Grant mentions, I can confirm that the packages exist as a field in the datastore_versions table, and this can be set only by the Trove Administrator when he is registering a new datastore version with trove (using the trove-manage utility). This is not a user facing field, and cannot be set through an API. Given this mitigation, I agree with opening the report up, and fixing this as part of security hardening. At this point it's likely that this will be tackled sometime during the Liberty time frame in Trove.

[Jeremy Stanley (fungi)]

Seems squarely within the class D security hardening arena to me, unless there's a reasonable expectation that Trove instances are in some way protected from malicious Trove administrators (which I don't think can be argued).

[Grant Murphy (gmurphy)]

Ok then. Lets open this up!