Skip to content

Commit

Permalink
Prohibit file injection writing to host filesystem
Browse files Browse the repository at this point in the history
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.

Note this requires adding the 'readlink' binary to the
nova sudoers file.

Fixes bug: 1031311, CVE-2012-3447
Change-Id: I7f7cdeeffadebae7451e1e13f73f1313a7df9c5c
  • Loading branch information
Pádraig Brady committed Aug 7, 2012
1 parent 1e21810 commit ed89587
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 1 deletion.
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 import disk
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._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 @@ -519,9 +519,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.py
Expand Up @@ -275,7 +275,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()
return absolute_path
Expand Down

0 comments on commit ed89587

Please sign in to comment.