diff --git a/kolla/image/tasks.py b/kolla/image/tasks.py index 5038c1d52d..091f41f65b 100644 --- a/kolla/image/tasks.py +++ b/kolla/image/tasks.py @@ -264,6 +264,14 @@ class BuildTask(EngineTask): def builder(self, image): + def _test_malicious_tarball(archive, path): + tar_file = tarfile.open(archive, 'r|gz') + for n in tar_file.getnames(): + if not os.path.abspath(os.path.join(path, n)).startswith(path): + tar_file.close() + self.logger.error(f'Unsafe filenames in archive {archive}') + raise ArchivingError + def make_an_archive(items, arcname, item_child_path=None): if not item_child_path: item_child_path = arcname @@ -277,8 +285,9 @@ class BuildTask(EngineTask): archives.append(archive_path) if archives: for archive in archives: + _test_malicious_tarball(archive, items_path) with tarfile.open(archive, 'r') as archive_tar: - archive_tar.extractall(path=items_path) + archive_tar.extractall(path=items_path) # nosec else: try: os.mkdir(items_path) diff --git a/kolla/image/unbuildable.py b/kolla/image/unbuildable.py index dc5183162e..c27eb9593e 100644 --- a/kolla/image/unbuildable.py +++ b/kolla/image/unbuildable.py @@ -24,6 +24,7 @@ UNBUILDABLE_IMAGES = { # Issues for SHA1 keys: # https://github.com/grafana/grafana/issues/41036 'centos': { + "bifrost-base", # EPEL-related breakage "hacluster-pcs", # Missing crmsh package "nova-spicehtml5proxy", # Missing spicehtml5 package "ovsdpdk", # Not supported on CentOS @@ -34,6 +35,7 @@ UNBUILDABLE_IMAGES = { }, 'rocky': { + "bifrost-base", # EPEL-related breakage "hacluster-pcs", # Missing crmsh package "nova-spicehtml5proxy", # Missing spicehtml5 package "ovsdpdk", # Not supported on CentOS diff --git a/kolla/tests/test_build.py b/kolla/tests/test_build.py index a4784ba86b..ce7896da54 100644 --- a/kolla/tests/test_build.py +++ b/kolla/tests/test_build.py @@ -14,6 +14,8 @@ import fixtures import os import requests import sys +import tarfile +import tempfile from unittest import mock from kolla.cmd import build as build_cmd @@ -289,6 +291,46 @@ class TasksTest(base.TestCase): else: self.assertIsNotNone(get_result) + @mock.patch.dict(os.environ, clear=True) + @mock.patch('docker.APIClient') + def test_malicious_tar(self, mock_client): + tmpdir = tempfile.mkdtemp() + file_name = 'test.txt' + archive_name = 'my_archive.tar.gz' + file_path = os.path.join(tmpdir, file_name) + archive_path = os.path.join(tmpdir, archive_name) + # Ensure the file is read/write by the creator only + saved_umask = os.umask(0o077) + + try: + with open(file_path, 'w') as f: + f.write('Hello') + + with tarfile.open(archive_path, 'w:gz') as tar: + tar.add(file_path, arcname='../test.txt') + + self.dc = mock_client + self.image.plugins = [{ + 'name': 'fake-image-base-plugin-test', + 'type': 'local', + 'enabled': True, + 'source': archive_path} + ] + + push_queue = mock.Mock() + builder = tasks.BuildTask(self.conf, self.image, push_queue) + builder.run() + self.assertFalse(builder.success) + + except IOError: + print('IOError') + else: + os.remove(file_path) + os.remove(archive_path) + finally: + os.umask(saved_umask) + os.rmdir(tmpdir) + @mock.patch('os.path.exists') @mock.patch('os.utime') @mock.patch('shutil.rmtree')