Skip to content

Commit 9516d00

Browse files
committed
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 3d008b7)
1 parent 3faff93 commit 9516d00

File tree

2 files changed

+52
-1
lines changed

2 files changed

+52
-1
lines changed

kolla/image/build.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,14 @@ def update_buildargs(self):
501501

502502
def builder(self, image):
503503

504+
def _test_malicious_tarball(archive, path):
505+
tar_file = tarfile.open(archive, 'r|gz')
506+
for n in tar_file.getnames():
507+
if not os.path.abspath(os.path.join(path, n)).startswith(path):
508+
tar_file.close()
509+
self.logger.error(f'Unsafe filenames in archive {archive}')
510+
raise ArchivingError
511+
504512
def make_an_archive(items, arcname, item_child_path=None):
505513
if not item_child_path:
506514
item_child_path = arcname
@@ -514,8 +522,9 @@ def make_an_archive(items, arcname, item_child_path=None):
514522
archives.append(archive_path)
515523
if archives:
516524
for archive in archives:
525+
_test_malicious_tarball(archive, items_path)
517526
with tarfile.open(archive, 'r') as archive_tar:
518-
archive_tar.extractall(path=items_path)
527+
archive_tar.extractall(path=items_path) # nosec
519528
else:
520529
try:
521530
os.mkdir(items_path)

kolla/tests/test_build.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
import os
1616
import requests
1717
import sys
18+
import tarfile
19+
import tempfile
1820
from unittest import mock
1921

2022
from kolla.cmd import build as build_cmd
@@ -303,6 +305,46 @@ def test_process_source(self, mock_get, mock_client,
303305
else:
304306
self.assertIsNotNone(get_result)
305307

308+
@mock.patch.dict(os.environ, clear=True)
309+
@mock.patch('docker.APIClient')
310+
def test_malicious_tar(self, mock_client):
311+
tmpdir = tempfile.mkdtemp()
312+
file_name = 'test.txt'
313+
archive_name = 'my_archive.tar.gz'
314+
file_path = os.path.join(tmpdir, file_name)
315+
archive_path = os.path.join(tmpdir, archive_name)
316+
# Ensure the file is read/write by the creator only
317+
saved_umask = os.umask(0o077)
318+
319+
try:
320+
with open(file_path, 'w') as f:
321+
f.write('Hello')
322+
323+
with tarfile.open(archive_path, 'w:gz') as tar:
324+
tar.add(file_path, arcname='../test.txt')
325+
326+
self.dc = mock_client
327+
self.image.plugins = [{
328+
'name': 'fake-image-base-plugin-test',
329+
'type': 'local',
330+
'enabled': True,
331+
'source': archive_path}
332+
]
333+
334+
push_queue = mock.Mock()
335+
builder = build.BuildTask(self.conf, self.image, push_queue)
336+
builder.run()
337+
self.assertFalse(builder.success)
338+
339+
except IOError:
340+
print('IOError')
341+
else:
342+
os.remove(file_path)
343+
os.remove(archive_path)
344+
finally:
345+
os.umask(saved_umask)
346+
os.rmdir(tmpdir)
347+
306348
@mock.patch('os.path.exists')
307349
@mock.patch('os.utime')
308350
@mock.patch('shutil.rmtree')

0 commit comments

Comments
 (0)