Several command injection vulnerabilities in guestagent/pkg

Bug #1434545 reported by Travis McPeak
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack DBaaS (Trove)
Triaged
Medium
Unassigned
OpenStack Security Advisory
Won't Fix
Undecided
Unassigned

Bug Description

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.

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

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 ?

Revision history for this message
Travis McPeak (travis-mcpeak) wrote :

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.

Revision history for this message
Travis McPeak (travis-mcpeak) wrote :

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

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

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.

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

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

Revision history for this message
Nikhil Manchanda (slicknik) wrote :

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.

Changed in trove:
status: New → Triaged
importance: Undecided → High
milestone: none → liberty-1
Revision history for this message
Jeremy Stanley (fungi) wrote :

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

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

Ok then. Lets open this up!

information type: Private Security → Public Security
Jeremy Stanley (fungi)
information type: Public Security → Public
tags: added: security
Changed in ossa:
status: Incomplete → Won't Fix
Anna Shen (ruiyuan-shen)
Changed in trove:
assignee: nobody → Anna Shen (ruiyuan-shen)
Changed in trove:
milestone: liberty-1 → liberty-2
Changed in trove:
milestone: liberty-2 → liberty-3
Changed in trove:
milestone: liberty-3 → ongoing
assignee: Anna Shen (ruiyuan-shen) → nobody
Amrith Kumar (amrith)
Changed in trove:
assignee: nobody → Amrith (amrith)
Amrith Kumar (amrith)
Changed in trove:
importance: High → Medium
Amrith Kumar (amrith)
Changed in trove:
assignee: Amrith Kumar (amrith) → nobody
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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