Prohibit file injection writing to host filesystem
This is a refinement of the previous fix in commit 2427d4a9
,
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
This commit is contained in:
parent
9d753f18e1
commit
ce4b2e27be
@ -99,22 +99,6 @@ class TestVirtDisk(test.TestCase):
|
||||
|
||||
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 @@ class TestVirtDisk(test.TestCase):
|
||||
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 @@ class XenAPIVMTestCase(stubs.XenAPITestBase):
|
||||
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,
|
||||
|
@ -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
|
||||
|
Loading…
Reference in New Issue
Block a user