Test for unsafe files in tarfile.extractall

In addition to that, mark bifrost unbuildable since it's
failing on EPEL 9 enablement.

Closes-Bug: #1990432

Change-Id: I650fcbc8f773fad8116338f6fb0cf7b4f4f17b33
(cherry picked from commit 3d008b7f5ec2a54d004e8e9370f303ef9dc7858b)
This commit is contained in:
Michal Nasiadka 2023-03-16 10:04:47 +00:00
parent 3faff93e8d
commit 9516d00f3b
2 changed files with 52 additions and 1 deletions

View File

@ -501,6 +501,14 @@ class BuildTask(DockerTask):
def builder(self, image): 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): def make_an_archive(items, arcname, item_child_path=None):
if not item_child_path: if not item_child_path:
item_child_path = arcname item_child_path = arcname
@ -514,8 +522,9 @@ class BuildTask(DockerTask):
archives.append(archive_path) archives.append(archive_path)
if archives: if archives:
for archive in archives: for archive in archives:
_test_malicious_tarball(archive, items_path)
with tarfile.open(archive, 'r') as archive_tar: with tarfile.open(archive, 'r') as archive_tar:
archive_tar.extractall(path=items_path) archive_tar.extractall(path=items_path) # nosec
else: else:
try: try:
os.mkdir(items_path) os.mkdir(items_path)

View File

@ -15,6 +15,8 @@ import itertools
import os import os
import requests import requests
import sys import sys
import tarfile
import tempfile
from unittest import mock from unittest import mock
from kolla.cmd import build as build_cmd from kolla.cmd import build as build_cmd
@ -303,6 +305,46 @@ class TasksTest(base.TestCase):
else: else:
self.assertIsNotNone(get_result) 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 = build.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.path.exists')
@mock.patch('os.utime') @mock.patch('os.utime')
@mock.patch('shutil.rmtree') @mock.patch('shutil.rmtree')