diff --git a/Doc/library/tarfile.rst b/Doc/library/tarfile.rst index 06eba01d21e5261..a28d8b906979661 100644 --- a/Doc/library/tarfile.rst +++ b/Doc/library/tarfile.rst @@ -1132,7 +1132,9 @@ reused in custom filters: path (after following symlinks) would end up outside the destination. This raises :class:`~tarfile.OutsideDestinationError`. - :ref:`Refuse ` to extract links (hard or soft) - whose name refers to the destination directory itself. + that would be made at the destination directory itself or outside it. + Only the directories leading to the link are resolved: a link replaces + whatever is at its own path rather than following it. This raises :class:`~tarfile.OutsideDestinationError`. Note that such files are refused even on platforms that do not support symbolic links. @@ -1152,6 +1154,9 @@ reused in custom filters: Filenames containing ``..`` components are now normalized. + Links that would be made at the destination directory itself, or + at an existing symbolic link outside it, are now refused. + .. function:: data_filter(member, path) Implements the ``'data'`` filter. diff --git a/Lib/tarfile.py b/Lib/tarfile.py index 57bba1bba54db35..ea205e0c1a56561 100644 --- a/Lib/tarfile.py +++ b/Lib/tarfile.py @@ -854,12 +854,19 @@ def _get_filtered_attrs(member, dest_path, for_data=True): strict=os.path.ALLOW_MISSING) if os.path.commonpath([target_path, dest_path]) != dest_path: raise OutsideDestinationError(member, target_path) - # A link member named after the destination directory itself - # would replace it with a (sym)link, redirecting the destination - # for all subsequent members. - if ((member.islnk() or member.issym()) - and os.path.normpath(name) == os.curdir): - raise OutsideDestinationError(member, target_path) + if member.islnk() or member.issym(): + # A link is made at its own path without following a link already + # there (that is unlinked first), so judge it by where it is made: + # resolve only the parent directory. The link must land strictly + # inside the destination; one that replaced the destination itself + # would redirect all subsequent members. + parent, leaf = os.path.split(name.rstrip('/' + os.sep)) + parent = os.path.realpath(os.path.join(dest_path, parent), + strict=os.path.ALLOW_MISSING) + link_path = os.path.normpath(os.path.join(parent, leaf)) + if (os.path.commonpath([link_path, dest_path]) != dest_path + or os.path.normcase(link_path) == os.path.normcase(dest_path)): + raise OutsideDestinationError(member, link_path) # Limit permissions (no high bits, and go-w) mode = member.mode if mode is not None: diff --git a/Lib/test/test_tarfile.py b/Lib/test/test_tarfile.py index 639fd0449a989ab..1ca58a95d76ef80 100644 --- a/Lib/test/test_tarfile.py +++ b/Lib/test/test_tarfile.py @@ -4676,6 +4676,82 @@ def test_link_at_destination(self): with self.check_context(arc.open(), filter): self.expect_exception(tarfile.OutsideDestinationError) + @symlink_test + @os_helper.skip_unless_symlink + def test_link_reaching_destination(self): + # gh-160863: link members whose names are not lexically '.' but + # that would still be made at the destination directory itself. + def via_parent_name(arc): + arc.add('../dest', symlink_to='dest') + + def via_symlink_to_dot(arc): + arc.add('d/') + arc.add('s', symlink_to='.') + arc.add('s/.', hardlink_to='d') + + def via_symlink_to_dot_symlink(arc): + arc.add('s', symlink_to='.') + arc.add('s/.', symlink_to='.') + + for make in (via_parent_name, via_symlink_to_dot, + via_symlink_to_dot_symlink): + with ArchiveMaker() as arc: + make(arc) + for filter in 'tar', 'data': + with self.subTest(make.__name__, filter=filter): + with self.check_context(arc.open(), filter): + self.expect_exception(tarfile.OutsideDestinationError) + + @symlink_test + @os_helper.skip_unless_symlink + def test_link_replacing_outer_symlink(self): + # gh-160863: a link member must not replace an existing symlink + # outside the destination, even one that points into it. + with ArchiveMaker() as arc: + arc.add('first.txt', content='1') + arc.add('../current', symlink_to='real') + arc.add('later.txt', content='2') + for filter in 'tar', 'data': + with self.subTest(filter=filter), \ + os_helper.temp_dir(self.outerdir): + real = self.outerdir / 'real' + current = self.outerdir / 'current' + real.mkdir() + current.symlink_to('real') + with arc.open() as tar: + with self.assertRaises(tarfile.OutsideDestinationError): + tar.extractall(current, filter=filter) + self.assertTrue(current.is_symlink()) + self.assertEqual(os.listdir(real), ['first.txt']) + + # The link's parent directory may also be reached through a + # symlink made earlier in the same archive. + with ArchiveMaker() as arc: + arc.add('s', symlink_to='..') + arc.add('s/outer', symlink_to='dest') + with os_helper.temp_dir(self.outerdir): + outer = self.outerdir / 'outer' + outer.symlink_to('dest') + with arc.open() as tar: + with self.assertRaises(tarfile.OutsideDestinationError): + tar.extractall(self.destdir, filter='tar') + self.assertTrue(outer.is_symlink()) + + @symlink_test + @os_helper.skip_unless_symlink + def test_link_to_dot_reextracted(self): + # A symlink to '.' resolves to the destination once it exists; + # extracting the same archive again must still replace it. + with ArchiveMaker() as arc: + arc.add('self', symlink_to='.') + for filter in 'tar', 'data': + with self.subTest(filter=filter), \ + os_helper.temp_dir(self.outerdir): + for _ in range(2): + with arc.open() as tar: + tar.extractall(self.destdir, filter=filter) + self.assertEqual(os.readlink(self.destdir / 'self'), '.') + @symlink_test @os_helper.skip_unless_symlink def test_empty_name_symlink_chain(self): diff --git a/Misc/NEWS.d/next/Security/2026-10-29-12-00-00.gh-issue-160863.zfGUag.rst b/Misc/NEWS.d/next/Security/2026-10-29-12-00-00.gh-issue-160863.zfGUag.rst new file mode 100644 index 000000000000000..e68de2463eb9f49 --- /dev/null +++ b/Misc/NEWS.d/next/Security/2026-10-29-12-00-00.gh-issue-160863.zfGUag.rst @@ -0,0 +1,5 @@ +:func:`tarfile.tar_filter` and :func:`tarfile.data_filter` now judge a link +member by where the link would be made, resolving only its parent directory. +This rejects links that reach the destination directory, or an existing +symbolic link outside it, through ``..`` components or earlier symlink +members, which :gh:`160097` had allowed past the ``'data'`` filter.