Skip to content

Commit 9b350df

Browse files
mnasiadkammalchuk
authored andcommitted
Test for unsafe files in tarfile.extractall
Closes-Bug: #1990432 Change-Id: I650fcbc8f773fad8116338f6fb0cf7b4f4f17b33 (cherry picked from commit 3d008b7)
1 parent e9f0a9a commit 9b350df

File tree

2 files changed

+53
-1
lines changed

2 files changed

+53
-1
lines changed

kolla/image/build.py

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

491491
def builder(self, image):
492492

493+
def _test_malicious_tarball(archive, path):
494+
tar_file = tarfile.open(archive, 'r|gz')
495+
for n in tar_file.getnames():
496+
if not os.path.abspath(os.path.join(path, n)).startswith(path):
497+
tar_file.close()
498+
self.logger.error(f'Unsafe filenames in archive {archive}')
499+
raise ArchivingError
500+
493501
def make_an_archive(items, arcname, item_child_path=None):
494502
if not item_child_path:
495503
item_child_path = arcname
@@ -502,8 +510,9 @@ def make_an_archive(items, arcname, item_child_path=None):
502510
archives.append(archive_path)
503511
if archives:
504512
for archive in archives:
513+
_test_malicious_tarball(archive, items_path)
505514
with tarfile.open(archive, 'r') as archive_tar:
506-
archive_tar.extractall(path=items_path)
515+
archive_tar.extractall(path=items_path) # nosec
507516
else:
508517
try:
509518
os.mkdir(items_path)

kolla/tests/test_build.py

Lines changed: 43 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
@@ -299,6 +301,47 @@ def test_process_source(self, mock_get, mock_client,
299301
else:
300302
self.assertIsNotNone(get_result)
301303

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

0 commit comments

Comments
 (0)