Skip to content

Commit 038a091

Browse files
[3.15] gh-155999: tarfile: handle a member that leaves the destination but comes back (GH-156000) (#156040)
gh-155999: `tarfile`: handle a member that leaves the destination but comes back (GH-156000) (cherry picked from commit 9768834) Co-authored-by: Stan Ulbrych <stan@python.org>
1 parent 3122687 commit 038a091

4 files changed

Lines changed: 34 additions & 0 deletions

File tree

‎Doc/library/tarfile.rst‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,6 +1107,10 @@ reused in custom filters:
11071107
paths (in case the name is absolute
11081108
even after stripping slashes, e.g. ``C:/foo`` on Windows).
11091109
This raises :class:`~tarfile.AbsolutePathError`.
1110+
- Normalize filenames (:attr:`TarInfo.name`) that contain ``..`` components
1111+
using :func:`os.path.normpath`.
1112+
Note that this removes internal ``..`` components, which may change the
1113+
meaning of the name if it traverses symbolic links.
11101114
- :ref:`Refuse <tarfile-extraction-refuse>` to extract files whose absolute
11111115
path (after following symlinks) would end up outside the destination.
11121116
This raises :class:`~tarfile.OutsideDestinationError`.
@@ -1115,6 +1119,10 @@ reused in custom filters:
11151119

11161120
Return the modified ``TarInfo`` member.
11171121

1122+
.. versionchanged:: next
1123+
1124+
Filenames containing ``..`` components are now normalized.
1125+
11181126
.. function:: data_filter(member, path)
11191127

11201128
Implements the ``'data'`` filter.

‎Lib/tarfile.py‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,13 @@ def _get_filtered_attrs(member, dest_path, for_data=True):
833833
# For example, 'C:/foo' on Windows.
834834
raise AbsolutePathError(member)
835835
# Ensure we stay in the destination
836+
if '..' in name.replace(os.sep, '/').split('/'):
837+
# Directories are created from the name as given, so a name that
838+
# leaves the destination part-way through would create them
839+
# outside it even if the resolved path stays inside.
840+
normalized = os.path.normpath(name)
841+
if normalized != name:
842+
name = new_attrs['name'] = normalized
836843
target_path = os.path.realpath(os.path.join(dest_path, name),
837844
strict=os.path.ALLOW_MISSING)
838845
if os.path.commonpath([target_path, dest_path]) != dest_path:

‎Lib/test/test_tarfile.py‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3968,6 +3968,20 @@ def test_absolute(self):
39683968
tarfile.AbsolutePathError,
39693969
"""['"].*escaped.evil['"] has an absolute path""")
39703970

3971+
def test_parent_dir_out_and_back(self):
3972+
# Test a member that leaves the destination and comes back.
3973+
# The containment check looks at the resolved path, which stays
3974+
# inside, but the intermediate directories are created from the
3975+
# name as given, which does not.
3976+
with ArchiveMaker() as arc:
3977+
arc.add(f'../escaped.evil/../{self.destdir.name}/sub/file',
3978+
content='content')
3979+
3980+
for filter in 'tar', 'data':
3981+
with self.subTest(filter):
3982+
with self.check_context(arc.open(), filter):
3983+
self.expect_file('sub/file', content='content')
3984+
39713985
@symlink_test
39723986
def test_parent_symlink(self):
39733987
# Test interplaying symlinks
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix the :mod:`tarfile` ``tar`` and ``data`` extraction filters creating
2+
directories outside the destination for members whose name leaves the
3+
destination and returns to it, such as ``../evil/../dest/sub/file``. The
4+
containment check used the resolved path, but intermediate directories were
5+
created from the name as given.

0 commit comments

Comments
 (0)