[OSSA 2014-012] Remote Code Execution in Sheepdog backend (CVE-2014-0162)

[Paul McMillan]

The Sheepdog backend for Glance appears to allow an attacker to remotely execute arbitrary code as the glance user.

https://github.com/openstack/glance/blob/9e9ce645e39d55b4da540b15b41f85bd2b4bd518/glance/store/sheepdog.py#L75

This code should be reworked so that it doesn't need shell=True. As it currently stands, it appears that an admin can insert or modify an image with a specially crafted id, which would trigger code execution. I don't immediately see a way for a non-admin user to trigger the injection, but the possibility does exist.

[Paul McMillan (paul-mcmillan)]

The current version of the patch looks good.

You don't need the map in this line:
        cmd.extend(map(str, params))
It can just be like this:
        cmd.extend(params)

Since the helper function already does that here:
https://github.com/openstack/glance/blob/master/glance/openstack/common/processutils.py#L137

Otherwise, this looks like a very good solution to the problem.

[Zhi Yan Liu (lzy-dev)]

@paul-mcmillan, done: "cmd.extend(map(str, params))" => "cmd.extend(params)".

[Tristan Cacqueray (tristan-cacqueray)]

@Paul McMillan I was able to reproduce this bug using location parameter but not with image id.

And for the affection version, the offending commit (1757e7e0) appeared in 2013.2.

Impact description draft #1:

Title: Remote code execution in Glance Sheepdog backend
Reporter: Paul McMillan (Nebula)
Products: Glance
Versions: 2013.2 versions up to 2013.2.2

Description:
Paul McMillan from Nebula reported a vulnerability in Glance Sheepdog backend. By using a specially crafted location, a user allowed to insert or modify a Glance image may trigger remote code execution resulting in Glance host unauthorized access. Only Glance setups featuring Sheepdog backend are affected. Note that it is enabled by default and should be removed from the known_stores list in glance-api.conf to be disabled.

[Zhi Yan Liu (lzy-dev)]

My patch based/depended on change https://review.openstack.org/#/c/80674 (on reviewing) which fixed bug 1292170, it makes the sheepdog test case be OK. Otherwise the Glance built-in testing will be failed after your patched the change.

[Thierry Carrez (ttx)]

Few minor remarks on the impact description:

"insert or modify Glance image metadata"
"may trigger code execution on the Glance host as the user the Glance service runs under. This may result in Glance host unauthorized access and further compromise of the Glance service."
"Glance setups enabling Sheepdog backend"

otherwise looks good to me.

[Tristan Cacqueray (tristan-cacqueray)]

@ttx Thanks!

Impact description draft #2:

Title: Remote code execution in Glance Sheepdog backend
Reporter: Paul McMillan (Nebula)
Products: Glance
Versions: 2013.2 versions up to 2013.2.2

Description:
Paul McMillan from Nebula reported a vulnerability in Glance Sheepdog backend. By using a specially crafted location, a user allowed to insert or modify Glance image metadata may trigger code execution on the Glance host as the user the Glance service runs under. This may result in Glance host unauthorized access and further compromise of the Glance service. Only Glance setups enabling Sheepdog backend are affected. Note that it is enabled by default and should be removed from the known_stores list in glance-api.conf to be disabled.

[Thierry Carrez (ttx)]

+1 on Impact description draft #2

[Mark Washenberger (markwash)]

Code looks good to me.
I'm not sure on the description, however, that we can actually just remove the Sheepdog store to disable it. Due to some unfortunate upgrade issues, we had to make it so that additional stores are configured on by default even if they are not specified in the list of "known stores" which might be a problem worthy of fixing on its own. Background is here:
https://bugs.launchpad.net/glance/+bug/1290969

[Tristan Cacqueray (tristan-cacqueray)]

@markwash well setting the known_stores manually without sheepdog disabled it on Havana. My icehouse devstack must have been outdated because now this does not prevent sheepdog backend to be disabled.

Nice catch thanks!

@ttx Here is a revised affected setup line:

Note that it is enabled by default and can only be disabled in Havana by removing the backend from the known_stores list in glance-api.conf.

[Mark Washenberger (markwash)]

Thanks Tristan. Comments sound correct for Havana.

[Zhi Yan Liu (lzy-dev)]

The dependency of the patch has gone, https://review.openstack.org/#/c/80674 got merged. IFY.

[Tristan Cacqueray (tristan-cacqueray)]

@Zhi Yan Liu Could you please attach another patch for Havana ?
The one in comment #7 doesn't not apply cleanly, 5 'Hunks' are currently rejected.

[Tristan Cacqueray (tristan-cacqueray)]

@Zhi Yan Liu About your fix, why did you renamed configure_add in configure, and removed the sheepdog.Store stubs in tests ? It seems to make thoses tests fail:

- glance.tests.functional.store.test_filesystem.TestFilesystemStore
- glance.tests.functional.store.test_http.TestHTTPStore
- glance.tests.unit.test_store_base.TestStoreBase
- glance.tests.unit.test_store_location.TestStoreLocation

Also, I think it misses a expect OSError after the processutils.execute. Without shell=True, execute raises OSError instead of ProcessExecutionError and it makes this test fail:

- glance.tests.functional.store.test_sheepdog.TestSheepdogStore

Do you have thoses errors as well ?

[Tristan Cacqueray (tristan-cacqueray)]

Here a version of the master patch that apply to havana and fix tests:

[Zhi Yan Liu (lzy-dev)]

@Tristan,

I can help make a patch version for Havana branch.

The reason for changing configure_add method to configure is that currently if store raise BadStoreConfiguration exception from configure_add method, the glance will just make this store be readonly instead of fully disable, but for the logic of sheepdog configure_add, it contains all essential initial configuration for the store which are necessary for the store be fully functional, so if any one of they get failed the store should be full block up instead of being readonly accessible.

I have removed stubs for sheepdog.Store.configure_add method, but I can run testing successful on my local development environment, including glance.tests.functional.store.test_sheepdog.TestSheepdogStore even there's not exception caching for ProcessExecutionError.

[Zhi Yan Liu (lzy-dev)]

@Tristan, and I'd like to do a minor change on the patch for following asap later (I'm in a business travel and will back to work soon)

1. adding ProcessExecutionError exception handling for non-shell execution to prevent potential issue, even I can't reproduce problem you met on glance.tests.functional.store.test_sheepdog.TestSheepdogStore case.

2. prepared a version for H branch.

Left message to me here as needed, and will check irc routinely as well.

[Thierry Carrez (ttx)]

This is a bit time-sensitive, we need the final patch up ASAP to be able to include it in Icehouse release.

[Tristan Cacqueray (tristan-cacqueray)]

Master patch in comment #22 is looking good and pass all tests.

Here is a backport of it to stable/havana, note that I had to remove the uuid check that is not available in Havana.

[Zhi Yan Liu (lzy-dev)]

Util is_uuid_like() is available in Havana in a different module: https://github.com/openstack/glance/blob/stable/havana/glance/openstack/common/uuidutils.py#L29 IFY.

[Mark Washenberger (markwash)]

+2 on master patch
-1 on havana patch, what's the deal with the test-requirements changes?

[Zhi Yan Liu (lzy-dev)]

@markwash, the change just works for my local network issue as a workaround, my/GFW bad.

[Tristan Cacqueray (tristan-cacqueray)]

Last stable/havana patch in comment #27 is passing run_tests.sh

[Tristan Cacqueray (tristan-cacqueray)]

The pre-OSSA have been sent.

Proposed public disclosure date/time:
2014-04-10 15:00 UTC

[Mark Washenberger (markwash)]

+2 havana patch

[Tristan Cacqueray (tristan-cacqueray)]

Master review: https://review.openstack.org/86622
stable review: https://review.openstack.org/86626
Milestone review: https://review.openstack.org/86625