Skip to content
Permalink
Browse files Browse the repository at this point in the history
Prohibit file injection writing to host filesystem
This is a refinement of the previous fix in commit 2427d4a,
which does the file name canonicalization as the root user.
This is required so that guest images could not for example,
protect malicious symlinks in a directory only readable by root.

Fixes bug: 1031311, CVE-2012-3447
Change-Id: I7f7cdeeffadebae7451e1e13f73f1313a7df9c5c
  • Loading branch information
Pádraig Brady committed Aug 7, 2012
1 parent 6c05f43 commit d9577ce
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 1 deletion.
4 changes: 4 additions & 0 deletions nova/rootwrap/compute.py
Expand Up @@ -189,6 +189,10 @@
# nova/virt/libvirt/utils.py: 'qemu-img'
filters.CommandFilter("/usr/bin/qemu-img", "root"),

# nova/virt/disk/api.py: 'readlink', '-e'
filters.CommandFilter("/usr/bin/readlink", "root"),
filters.CommandFilter("/bin/readlink", "root"),

# nova/virt/disk/api.py: 'touch', target
filters.CommandFilter("/usr/bin/touch", "root"),

Expand Down
12 changes: 12 additions & 0 deletions nova/tests/test_virt.py
Expand Up @@ -18,6 +18,7 @@
from nova import exception
from nova import flags
from nova import test
from nova import utils
from nova.virt.disk import api as disk_api
from nova.virt import driver

Expand Down Expand Up @@ -86,6 +87,17 @@ def test_swap_is_usable(self):


class TestVirtDisk(test.TestCase):
def setUp(self):
super(TestVirtDisk, self).setUp()

real_execute = utils.execute

def nonroot_execute(*cmd_parts, **kwargs):
kwargs.pop('run_as_root', None)
return real_execute(*cmd_parts, **kwargs)

self.stubs.Set(utils, 'execute', nonroot_execute)

def test_check_safe_path(self):
ret = disk_api._join_and_check_path_within_fs('/foo', 'etc',
'something.conf')
Expand Down
4 changes: 4 additions & 0 deletions nova/tests/test_xenapi.py
Expand Up @@ -597,9 +597,13 @@ def _tee_handler(cmd, **kwargs):
self._tee_executed = True
return '', ''

def _readlink_handler(cmd_parts, **kwargs):
return os.path.realpath(cmd_parts[2]), ''

fake_utils.fake_execute_set_repliers([
# Capture the tee .../etc/network/interfaces command
(r'tee.*interfaces', _tee_handler),
(r'readlink -nm.*', _readlink_handler),
])
self._test_spawn(glance_stubs.FakeGlance.IMAGE_MACHINE,
glance_stubs.FakeGlance.IMAGE_KERNEL,
Expand Down
4 changes: 3 additions & 1 deletion nova/virt/disk/api.py
Expand Up @@ -314,7 +314,9 @@ def _join_and_check_path_within_fs(fs, *args):
mounted guest fs. Trying to be clever and specifying a
path with '..' in it will hit this safeguard.
'''
absolute_path = os.path.realpath(os.path.join(fs, *args))
absolute_path, _err = utils.execute('readlink', '-nm',
os.path.join(fs, *args),
run_as_root=True)
if not absolute_path.startswith(os.path.realpath(fs) + '/'):
raise exception.Invalid(_('injected file path not valid'))
return absolute_path
Expand Down

0 comments on commit d9577ce

Please sign in to comment.