From 2427d4a99bed35baefd8f17ba422cb7aae8dcca7 Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Wed, 20 Jun 2012 11:13:13 -0400 Subject: [PATCH] Prevent file injection writing to host filesystem. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix bug 1015531, CVE-2012-3360, CVE-2012-3361 This patch prevents the file injection code from writing into the host filesystem if a user specifies a path for the injected file that contains '..'. The check is to make sure that the final normalized path that is about to be written to is within the mounted guest filesystem. Signed-off-by: Russell Bryant Signed-off-by: Pádraig Brady Signed-off-by: Mark McLoughlin Change-Id: I658cd12fd319cee91eb9544cdf53c862c5d2c560 --- nova/tests/test_virt.py | 20 ++++++++++++ nova/virt/disk/api.py | 71 ++++++++++++++++++++++++++--------------- 2 files changed, 66 insertions(+), 25 deletions(-) diff --git a/nova/tests/test_virt.py b/nova/tests/test_virt.py index 388f075afb44..c4aa828ad0de 100644 --- a/nova/tests/test_virt.py +++ b/nova/tests/test_virt.py @@ -15,8 +15,10 @@ # License for the specific language governing permissions and limitations # under the License. +from nova import exception from nova import flags from nova import test +from nova.virt.disk import api as disk_api from nova.virt import driver FLAGS = flags.FLAGS @@ -81,3 +83,21 @@ class TestVirtDriver(test.TestCase): 'swap_size': 0})) self.assertTrue(driver.swap_is_usable({'device_name': '/dev/sdb', 'swap_size': 1})) + + +class TestVirtDisk(test.TestCase): + 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') diff --git a/nova/virt/disk/api.py b/nova/virt/disk/api.py index 42d9d916260d..3e66cd2a7b8c 100644 --- a/nova/virt/disk/api.py +++ b/nova/virt/disk/api.py @@ -311,20 +311,39 @@ def inject_data_into_fs(fs, key, net, metadata, admin_password, execute): _inject_admin_password_into_fs(admin_password, fs, execute=execute) -def _inject_file_into_fs(fs, path, contents): - absolute_path = os.path.join(fs, path.lstrip('/')) +def _join_and_check_path_within_fs(fs, *args): + '''os.path.join() with safety check for injected file paths. + + Join the supplied path components and make sure that the + resulting path we are injecting into is within the + 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)) + if not absolute_path.startswith(os.path.realpath(fs) + '/'): + raise exception.Invalid(_('injected file path not valid')) + return absolute_path + + +def _inject_file_into_fs(fs, path, contents, append=False): + absolute_path = _join_and_check_path_within_fs(fs, path.lstrip('/')) + parent_dir = os.path.dirname(absolute_path) utils.execute('mkdir', '-p', parent_dir, run_as_root=True) - utils.execute('tee', absolute_path, process_input=contents, - run_as_root=True) + + args = [] + if append: + args.append('-a') + args.append(absolute_path) + + kwargs = dict(process_input=contents, run_as_root=True) + + utils.execute('tee', *args, **kwargs) def _inject_metadata_into_fs(metadata, fs, execute=None): - metadata_path = os.path.join(fs, "meta.js") metadata = dict([(m.key, m.value) for m in metadata]) - - utils.execute('tee', metadata_path, - process_input=jsonutils.dumps(metadata), run_as_root=True) + _inject_file_into_fs(fs, 'meta.js', jsonutils.dumps(metadata)) def _inject_key_into_fs(key, fs, execute=None): @@ -333,20 +352,22 @@ def _inject_key_into_fs(key, fs, execute=None): key is an ssh key string. fs is the path to the base of the filesystem into which to inject the key. """ - sshdir = os.path.join(fs, 'root', '.ssh') + sshdir = _join_and_check_path_within_fs(fs, 'root', '.ssh') utils.execute('mkdir', '-p', sshdir, run_as_root=True) utils.execute('chown', 'root', sshdir, run_as_root=True) utils.execute('chmod', '700', sshdir, run_as_root=True) - keyfile = os.path.join(sshdir, 'authorized_keys') - key_data = [ + + keyfile = os.path.join('root', '.ssh', 'authorized_keys') + + key_data = ''.join([ '\n', '# The following ssh key was injected by Nova', '\n', key.strip(), '\n', - ] - utils.execute('tee', '-a', keyfile, - process_input=''.join(key_data), run_as_root=True) + ]) + + _inject_file_into_fs(fs, keyfile, key_data, append=True) def _inject_net_into_fs(net, fs, execute=None): @@ -354,12 +375,13 @@ def _inject_net_into_fs(net, fs, execute=None): net is the contents of /etc/network/interfaces. """ - netdir = os.path.join(os.path.join(fs, 'etc'), 'network') + netdir = _join_and_check_path_within_fs(fs, 'etc', 'network') utils.execute('mkdir', '-p', netdir, run_as_root=True) utils.execute('chown', 'root:root', netdir, run_as_root=True) utils.execute('chmod', 755, netdir, run_as_root=True) - netfile = os.path.join(netdir, 'interfaces') - utils.execute('tee', netfile, process_input=net, run_as_root=True) + + netfile = os.path.join('etc', 'network', 'interfaces') + _inject_file_into_fs(fs, netfile, net) def _inject_admin_password_into_fs(admin_passwd, fs, execute=None): @@ -384,16 +406,15 @@ def _inject_admin_password_into_fs(admin_passwd, fs, execute=None): fd, tmp_shadow = tempfile.mkstemp() os.close(fd) - utils.execute('cp', os.path.join(fs, 'etc', 'passwd'), tmp_passwd, - run_as_root=True) - utils.execute('cp', os.path.join(fs, 'etc', 'shadow'), tmp_shadow, - run_as_root=True) + passwd_path = _join_and_check_path_within_fs(fs, 'etc', 'passwd') + shadow_path = _join_and_check_path_within_fs(fs, 'etc', 'shadow') + + utils.execute('cp', passwd_path, tmp_passwd, run_as_root=True) + utils.execute('cp', shadow_path, tmp_shadow, run_as_root=True) _set_passwd(admin_user, admin_passwd, tmp_passwd, tmp_shadow) - utils.execute('cp', tmp_passwd, os.path.join(fs, 'etc', 'passwd'), - run_as_root=True) + utils.execute('cp', tmp_passwd, passwd_path, run_as_root=True) os.unlink(tmp_passwd) - utils.execute('cp', tmp_shadow, os.path.join(fs, 'etc', 'shadow'), - run_as_root=True) + utils.execute('cp', tmp_shadow, shadow_path, run_as_root=True) os.unlink(tmp_shadow)