Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit b4368b5

Browse filesBrowse files
encukouvstinnerfrenzymadness
authored
[3.8] gh-107845: Fix symlink handling for tarfile.data_filter (GH-107846) (#108279)
(cherry picked from commit acbd3f9) Co-authored-by: Petr Viktorin <encukou@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Lumír 'Frenzy' Balhar <frenzy.madness@gmail.com>
1 parent a5d1571 commit b4368b5
Copy full SHA for b4368b5

File tree

Expand file treeCollapse file tree

4 files changed

+154
-9
lines changed
Filter options
Expand file treeCollapse file tree

4 files changed

+154
-9
lines changed

‎Doc/library/tarfile.rst

Copy file name to clipboardExpand all lines: Doc/library/tarfile.rst
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -725,6 +725,11 @@ A ``TarInfo`` object has the following public data attributes:
725725
Name of the target file name, which is only present in :class:`TarInfo` objects
726726
of type :const:`LNKTYPE` and :const:`SYMTYPE`.
727727

728+
For symbolic links (``SYMTYPE``), the *linkname* is relative to the directory
729+
that contains the link.
730+
For hard links (``LNKTYPE``), the *linkname* is relative to the root of
731+
the archive.
732+
728733

729734
.. attribute:: TarInfo.uid
730735
:type: int

‎Lib/tarfile.py

Copy file name to clipboardExpand all lines: Lib/tarfile.py
+9-2Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -740,7 +740,7 @@ def __init__(self, tarinfo):
740740
class AbsoluteLinkError(FilterError):
741741
def __init__(self, tarinfo):
742742
self.tarinfo = tarinfo
743-
super().__init__(f'{tarinfo.name!r} is a symlink to an absolute path')
743+
super().__init__(f'{tarinfo.name!r} is a link to an absolute path')
744744

745745
class LinkOutsideDestinationError(FilterError):
746746
def __init__(self, tarinfo, path):
@@ -800,7 +800,14 @@ def _get_filtered_attrs(member, dest_path, for_data=True):
800800
if member.islnk() or member.issym():
801801
if os.path.isabs(member.linkname):
802802
raise AbsoluteLinkError(member)
803-
target_path = os.path.realpath(os.path.join(dest_path, member.linkname))
803+
if member.issym():
804+
target_path = os.path.join(dest_path,
805+
os.path.dirname(name),
806+
member.linkname)
807+
else:
808+
target_path = os.path.join(dest_path,
809+
member.linkname)
810+
target_path = os.path.realpath(target_path)
804811
if os.path.commonpath([target_path, dest_path]) != dest_path:
805812
raise LinkOutsideDestinationError(member, target_path)
806813
return new_attrs

‎Lib/test/test_tarfile.py

Copy file name to clipboardExpand all lines: Lib/test/test_tarfile.py
+137-7Lines changed: 137 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2984,10 +2984,12 @@ def __exit__(self, *exc):
29842984
self.bio = None
29852985

29862986
def add(self, name, *, type=None, symlink_to=None, hardlink_to=None,
2987-
mode=None, **kwargs):
2987+
mode=None, size=None, **kwargs):
29882988
"""Add a member to the test archive. Call within `with`."""
29892989
name = str(name)
29902990
tarinfo = tarfile.TarInfo(name).replace(**kwargs)
2991+
if size is not None:
2992+
tarinfo.size = size
29912993
if mode:
29922994
tarinfo.mode = _filemode_to_int(mode)
29932995
if symlink_to is not None:
@@ -3051,7 +3053,8 @@ def check_context(self, tar, filter):
30513053
raise self.raised_exception
30523054
self.assertEqual(self.expected_paths, set())
30533055

3054-
def expect_file(self, name, type=None, symlink_to=None, mode=None):
3056+
def expect_file(self, name, type=None, symlink_to=None, mode=None,
3057+
size=None):
30553058
"""Check a single file. See check_context."""
30563059
if self.raised_exception:
30573060
raise self.raised_exception
@@ -3085,6 +3088,8 @@ def expect_file(self, name, type=None, symlink_to=None, mode=None):
30853088
self.assertTrue(path.is_fifo())
30863089
else:
30873090
raise NotImplementedError(type)
3091+
if size is not None:
3092+
self.assertEqual(path.stat().st_size, size)
30883093
for parent in path.parents:
30893094
self.expected_paths.discard(parent)
30903095

@@ -3130,8 +3135,15 @@ def test_parent_symlink(self):
31303135
# Test interplaying symlinks
31313136
# Inspired by 'dirsymlink2a' in jwilk/traversal-archives
31323137
with ArchiveMaker() as arc:
3138+
3139+
# `current` links to `.` which is both:
3140+
# - the destination directory
3141+
# - `current` itself
31333142
arc.add('current', symlink_to='.')
3143+
3144+
# effectively points to ./../
31343145
arc.add('parent', symlink_to='current/..')
3146+
31353147
arc.add('parent/evil')
31363148

31373149
if support.can_symlink():
@@ -3172,9 +3184,46 @@ def test_parent_symlink(self):
31723184
def test_parent_symlink2(self):
31733185
# Test interplaying symlinks
31743186
# Inspired by 'dirsymlink2b' in jwilk/traversal-archives
3187+
3188+
# Posix and Windows have different pathname resolution:
3189+
# either symlink or a '..' component resolve first.
3190+
# Let's see which we are on.
3191+
if support.can_symlink():
3192+
testpath = os.path.join(TEMPDIR, 'resolution_test')
3193+
os.mkdir(testpath)
3194+
3195+
# testpath/current links to `.` which is all of:
3196+
# - `testpath`
3197+
# - `testpath/current`
3198+
# - `testpath/current/current`
3199+
# - etc.
3200+
os.symlink('.', os.path.join(testpath, 'current'))
3201+
3202+
# we'll test where `testpath/current/../file` ends up
3203+
with open(os.path.join(testpath, 'current', '..', 'file'), 'w'):
3204+
pass
3205+
3206+
if os.path.exists(os.path.join(testpath, 'file')):
3207+
# Windows collapses 'current\..' to '.' first, leaving
3208+
# 'testpath\file'
3209+
dotdot_resolves_early = True
3210+
elif os.path.exists(os.path.join(testpath, '..', 'file')):
3211+
# Posix resolves 'current' to '.' first, leaving
3212+
# 'testpath/../file'
3213+
dotdot_resolves_early = False
3214+
else:
3215+
raise AssertionError('Could not determine link resolution')
3216+
31753217
with ArchiveMaker() as arc:
3218+
3219+
# `current` links to `.` which is both the destination directory
3220+
# and `current` itself
31763221
arc.add('current', symlink_to='.')
3222+
3223+
# `current/parent` is also available as `./parent`,
3224+
# and effectively points to `./../`
31773225
arc.add('current/parent', symlink_to='..')
3226+
31783227
arc.add('parent/evil')
31793228

31803229
with self.check_context(arc.open(), 'fully_trusted'):
@@ -3188,6 +3237,7 @@ def test_parent_symlink2(self):
31883237

31893238
with self.check_context(arc.open(), 'tar'):
31903239
if support.can_symlink():
3240+
# Fail when extracting a file outside destination
31913241
self.expect_exception(
31923242
tarfile.OutsideDestinationError,
31933243
"'parent/evil' would be extracted to "
@@ -3198,10 +3248,24 @@ def test_parent_symlink2(self):
31983248
self.expect_file('parent/evil')
31993249

32003250
with self.check_context(arc.open(), 'data'):
3201-
self.expect_exception(
3202-
tarfile.LinkOutsideDestinationError,
3203-
"""'current/parent' would link to ['"].*['"], """
3204-
+ "which is outside the destination")
3251+
if support.can_symlink():
3252+
if dotdot_resolves_early:
3253+
# Fail when extracting a file outside destination
3254+
self.expect_exception(
3255+
tarfile.OutsideDestinationError,
3256+
"'parent/evil' would be extracted to "
3257+
+ """['"].*evil['"], which is outside """
3258+
+ "the destination")
3259+
else:
3260+
# Fail as soon as we have a symlink outside the destination
3261+
self.expect_exception(
3262+
tarfile.LinkOutsideDestinationError,
3263+
"'current/parent' would link to "
3264+
+ """['"].*outerdir['"], which is outside """
3265+
+ "the destination")
3266+
else:
3267+
self.expect_file('current/')
3268+
self.expect_file('parent/evil')
32053269

32063270
def test_absolute_symlink(self):
32073271
# Test symlink to an absolute path
@@ -3230,11 +3294,29 @@ def test_absolute_symlink(self):
32303294
with self.check_context(arc.open(), 'data'):
32313295
self.expect_exception(
32323296
tarfile.AbsoluteLinkError,
3233-
"'parent' is a symlink to an absolute path")
3297+
"'parent' is a link to an absolute path")
3298+
3299+
def test_absolute_hardlink(self):
3300+
# Test hardlink to an absolute path
3301+
# Inspired by 'dirsymlink' in https://github.com/jwilk/traversal-archives
3302+
with ArchiveMaker() as arc:
3303+
arc.add('parent', hardlink_to=self.outerdir / 'foo')
3304+
3305+
with self.check_context(arc.open(), 'fully_trusted'):
3306+
self.expect_exception(KeyError, ".*foo. not found")
3307+
3308+
with self.check_context(arc.open(), 'tar'):
3309+
self.expect_exception(KeyError, ".*foo. not found")
3310+
3311+
with self.check_context(arc.open(), 'data'):
3312+
self.expect_exception(
3313+
tarfile.AbsoluteLinkError,
3314+
"'parent' is a link to an absolute path")
32343315

32353316
def test_sly_relative0(self):
32363317
# Inspired by 'relative0' in jwilk/traversal-archives
32373318
with ArchiveMaker() as arc:
3319+
# points to `../../tmp/moo`
32383320
arc.add('../moo', symlink_to='..//tmp/moo')
32393321

32403322
try:
@@ -3284,6 +3366,54 @@ def test_sly_relative2(self):
32843366
+ """['"].*moo['"], which is outside the """
32853367
+ "destination")
32863368

3369+
def test_deep_symlink(self):
3370+
# Test that symlinks and hardlinks inside a directory
3371+
# point to the correct file (`target` of size 3).
3372+
# If links aren't supported we get a copy of the file.
3373+
with ArchiveMaker() as arc:
3374+
arc.add('targetdir/target', size=3)
3375+
# a hardlink's linkname is relative to the archive
3376+
arc.add('linkdir/hardlink', hardlink_to=os.path.join(
3377+
'targetdir', 'target'))
3378+
# a symlink's linkname is relative to the link's directory
3379+
arc.add('linkdir/symlink', symlink_to=os.path.join(
3380+
'..', 'targetdir', 'target'))
3381+
3382+
for filter in 'tar', 'data', 'fully_trusted':
3383+
with self.check_context(arc.open(), filter):
3384+
self.expect_file('targetdir/target', size=3)
3385+
self.expect_file('linkdir/hardlink', size=3)
3386+
if support.can_symlink():
3387+
self.expect_file('linkdir/symlink', size=3,
3388+
symlink_to='../targetdir/target')
3389+
else:
3390+
self.expect_file('linkdir/symlink', size=3)
3391+
3392+
def test_chains(self):
3393+
# Test chaining of symlinks/hardlinks.
3394+
# Symlinks are created before the files they point to.
3395+
with ArchiveMaker() as arc:
3396+
arc.add('linkdir/symlink', symlink_to='hardlink')
3397+
arc.add('symlink2', symlink_to=os.path.join(
3398+
'linkdir', 'hardlink2'))
3399+
arc.add('targetdir/target', size=3)
3400+
arc.add('linkdir/hardlink', hardlink_to='targetdir/target')
3401+
arc.add('linkdir/hardlink2', hardlink_to='linkdir/symlink')
3402+
3403+
for filter in 'tar', 'data', 'fully_trusted':
3404+
with self.check_context(arc.open(), filter):
3405+
self.expect_file('targetdir/target', size=3)
3406+
self.expect_file('linkdir/hardlink', size=3)
3407+
self.expect_file('linkdir/hardlink2', size=3)
3408+
if support.can_symlink():
3409+
self.expect_file('linkdir/symlink', size=3,
3410+
symlink_to='hardlink')
3411+
self.expect_file('symlink2', size=3,
3412+
symlink_to='linkdir/hardlink2')
3413+
else:
3414+
self.expect_file('linkdir/symlink', size=3)
3415+
self.expect_file('symlink2', size=3)
3416+
32873417
def test_modes(self):
32883418
# Test how file modes are extracted
32893419
# (Note that the modes are ignored on platforms without working chmod)

‎Misc/NEWS.d/next/Library/2023-08-10-17-36-22.gh-issue-107845.dABiMJ.rst

Copy file name to clipboard
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:func:`tarfile.data_filter` now takes the location of symlinks into account
2+
when determining their target, so it will no longer reject some valid
3+
tarballs with ``LinkOutsideDestinationError``.

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.