Permalink
Browse files

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
Pádraig Brady committed Jul 31, 2012
1 parent 9d753f1 commit ce4b2e27be45a85b310237615c47eb53f37bb5f3
Showing with 36 additions and 17 deletions.
  1. +29 −16 nova/tests/test_virt.py
  2. +4 −0 nova/tests/test_xenapi.py
  3. +3 −1 nova/virt/disk/api.py
View
@@ -99,22 +99,6 @@ def fake_execute(*cmd, **kwargs):
self.stubs.Set(utils, 'execute', fake_execute)
def test_check_safe_path(self):
ret = disk_api._join_and_check_path_within_fs('/foo', 'etc',
'something.conf')
self.assertEquals(ret, '/foo/etc/something.conf')
def test_check_unsafe_path(self):
self.assertRaises(exception.Invalid,
disk_api._join_and_check_path_within_fs,
'/foo', 'etc/../../../something.conf')
def test_inject_files_with_bad_path(self):
self.assertRaises(exception.Invalid,
disk_api._inject_file_into_fs,
'/tmp', '/etc/../../../../etc/passwd',
'hax')
def test_lxc_destroy_container(self):
def proc_mounts(self, mount_point):
@@ -165,3 +149,32 @@ def proc_mounts(self, mount_point):
self.executes.pop()
self.assertEqual(self.executes, expected_commands)
class TestVirtDiskPaths(test.TestCase):
def setUp(self):
super(TestVirtDiskPaths, 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')
self.assertEquals(ret, '/foo/etc/something.conf')
def test_check_unsafe_path(self):
self.assertRaises(exception.Invalid,
disk_api._join_and_check_path_within_fs,
'/foo', 'etc/../../../something.conf')
def test_inject_files_with_bad_path(self):
self.assertRaises(exception.Invalid,
disk_api._inject_file_into_fs,
'/tmp', '/etc/../../../../etc/passwd',
'hax')
@@ -677,9 +677,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(IMAGE_MACHINE,
IMAGE_KERNEL,
View
@@ -363,7 +363,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

0 comments on commit ce4b2e2

Please sign in to comment.