Skip to content

Commit 111b538

Browse files
mundurmundur
authored andcommitted
Reject TAR links from untrusted archives
1 parent 925482c commit 111b538

2 files changed

Lines changed: 59 additions & 0 deletions

File tree

src/clusterfuzz/_internal/system/archive.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,14 @@ def extract(self,
295295
output_directory = os.path.realpath(path)
296296

297297
if not trusted:
298+
tar_member = self._archive.getmember(member)
299+
if tar_member.issym() or tar_member.islnk():
300+
logs.error(
301+
'Link member attempted while unpacking archive %s '
302+
'(member=%s, link target=%s). Aborting.' %
303+
(self._archive_path, member, tar_member.linkname))
304+
return None
305+
298306
if (_is_attempting_path_traversal(self._archive_path, output_directory,
299307
member)):
300308
return None

src/clusterfuzz/_internal/tests/core/system/archive_test.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,57 @@ def test_unpack_absolute_path_traversal(self):
8686
result = reader.extract_all(output_directory, trusted=False)
8787
self.assertFalse(result)
8888

89+
def test_unpack_tar_links_fails_when_untrusted(self):
90+
"""Test that unpacking TAR symbolic and hard links fails when untrusted."""
91+
with tempfile.NamedTemporaryFile(suffix='.tar') as tmp_tar_file:
92+
with tarfile.open(tmp_tar_file.name, 'w') as tar:
93+
for name, member_type, linkname in [
94+
('absolute-symlink', tarfile.SYMTYPE, '/tmp/pwned'),
95+
('relative-hardlink', tarfile.LNKTYPE, '../pwned'),
96+
]:
97+
tarinfo = tarfile.TarInfo(name=name)
98+
tarinfo.type = member_type
99+
tarinfo.linkname = linkname
100+
tar.addfile(tarinfo)
101+
102+
output_directory = tempfile.mkdtemp()
103+
self.addCleanup(shell.remove_directory, output_directory)
104+
105+
with archive.open(tmp_tar_file.name) as reader:
106+
result = reader.extract_all(output_directory, trusted=False)
107+
108+
self.assertFalse(result)
109+
self.assertFalse(
110+
os.path.lexists(os.path.join(output_directory, 'absolute-symlink')))
111+
self.assertFalse(
112+
os.path.lexists(os.path.join(output_directory, 'relative-hardlink')))
113+
114+
def test_unpack_tar_link_succeeds_when_trusted(self):
115+
"""Test that trusted TAR archives can still contain symbolic links."""
116+
with tempfile.NamedTemporaryFile(suffix='.tar') as tmp_tar_file:
117+
with tarfile.open(tmp_tar_file.name, 'w') as tar:
118+
file_data = b'contents'
119+
tarinfo = tarfile.TarInfo(name='target')
120+
tarinfo.size = len(file_data)
121+
tar.addfile(tarinfo, io.BytesIO(file_data))
122+
123+
tarinfo = tarfile.TarInfo(name='link')
124+
tarinfo.type = tarfile.SYMTYPE
125+
tarinfo.linkname = 'target'
126+
tar.addfile(tarinfo)
127+
128+
output_directory = tempfile.mkdtemp()
129+
self.addCleanup(shell.remove_directory, output_directory)
130+
131+
with archive.open(tmp_tar_file.name) as reader:
132+
result = reader.extract_all(output_directory, trusted=True)
133+
134+
link_path = os.path.join(output_directory, 'link')
135+
self.assertTrue(result)
136+
self.assertTrue(os.path.islink(link_path))
137+
with open(link_path, 'rb') as link_file:
138+
self.assertEqual(link_file.read(), b'contents')
139+
89140
def test_zip_extract_permissions_mocked_chmod(self):
90141
"""Test zip extraction only calls chmod when permissions change or execute bit is set."""
91142
helpers.patch(self, ['os.chmod'])

0 commit comments

Comments
 (0)