Skip to content

Commit 23159eb

Browse files
gpsheadmiss-islington
authored andcommitted
gh-160506: Fix ntpath.realpath() for broken links via directory links (GH-160507)
Resolve the directory containing a broken relative symlink before joining the link target to it, so that '..' in the target is applied to the real directory. ANT-2026-27II3A20 (cherry picked from commit 65665a4) Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com>
1 parent 42155a0 commit 23159eb

4 files changed

Lines changed: 99 additions & 5 deletions

File tree

‎Lib/ntpath.py‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,16 @@ def _readlink_deep(path, ignored_error=OSError):
594594
if not islink(old_path):
595595
path = old_path
596596
break
597-
path = normpath(join(dirname(old_path), path))
597+
# Resolve the directory holding the link first, so that
598+
# '..' in the target is not applied to a directory link
599+
# in old_path.
600+
link_dir = dirname(old_path)
601+
try:
602+
link_dir = _getfinalpathname(link_dir)
603+
except ignored_error as ex:
604+
if ex.winerror not in allowed_winerror:
605+
raise
606+
path = normpath(join(link_dir, path))
598607
except ignored_error as ex:
599608
if ex.winerror in allowed_winerror:
600609
break

‎Lib/test/test_ntpath.py‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -682,9 +682,9 @@ def test_realpath_broken_symlinks(self):
682682
self.assertPathEqual(ntpath.realpath(r"broken1\baz"),
683683
ABSTFN + r"\broken\bar\baz")
684684
self.assertPathEqual(ntpath.realpath("broken2"),
685-
ABSTFN + r"\self\self\missing")
685+
ABSTFN + r"\missing")
686686
self.assertPathEqual(ntpath.realpath("broken3"),
687-
ABSTFN + r"\subdir\parent\subdir\parent\missing")
687+
ABSTFN + r"\missing")
688688
self.assertPathEqual(ntpath.realpath("broken4"),
689689
ABSTFN + r"\missing")
690690
self.assertPathEqual(ntpath.realpath("broken5"),
@@ -699,14 +699,39 @@ def test_realpath_broken_symlinks(self):
699699
self.assertPathEqual(ntpath.realpath(rb"broken1\baz"),
700700
os.fsencode(ABSTFN + r"\broken\bar\baz"))
701701
self.assertPathEqual(ntpath.realpath(b"broken2"),
702-
os.fsencode(ABSTFN + r"\self\self\missing"))
702+
os.fsencode(ABSTFN + r"\missing"))
703703
self.assertPathEqual(ntpath.realpath(rb"broken3"),
704-
os.fsencode(ABSTFN + r"\subdir\parent\subdir\parent\missing"))
704+
os.fsencode(ABSTFN + r"\missing"))
705705
self.assertPathEqual(ntpath.realpath(b"broken4"),
706706
os.fsencode(ABSTFN + r"\missing"))
707707
self.assertPathEqual(ntpath.realpath(b"broken5"),
708708
os.fsencode(ABSTFN + r"\missing"))
709709

710+
@os_helper.skip_unless_symlink
711+
@unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname')
712+
@_parameterize({}, {'strict': ALL_BUT_LAST}, {'strict': ALLOW_MISSING})
713+
def test_realpath_broken_symlink_in_linked_dir(self, kwargs):
714+
# A relative target is resolved against the directory containing
715+
# the link, not against the path by which the link was reached.
716+
ABSTFN = ntpath.abspath(os_helper.TESTFN)
717+
os.makedirs(ABSTFN + r"\sub")
718+
self.addCleanup(os_helper.rmtree, ABSTFN)
719+
os.symlink(".", ABSTFN + r"\sub\alias", target_is_directory=True)
720+
os.symlink(r"..\missing", ABSTFN + r"\sub\broken")
721+
os.symlink(r"alias\alias\broken", ABSTFN + r"\sub\broken2")
722+
723+
expected = ABSTFN + r"\missing"
724+
for name in "broken", r"alias\broken", "broken2", r"alias\broken2":
725+
path = ABSTFN + "\\sub\\" + name
726+
with self.subTest(name=name):
727+
self.assertPathEqual(ntpath.realpath(path, **kwargs), expected)
728+
self.assertPathEqual(
729+
ntpath.realpath(os.fsencode(path), **kwargs),
730+
os.fsencode(expected))
731+
self.assertPathEqual(
732+
ntpath.realpath("\\\\?\\" + path, **kwargs),
733+
"\\\\?\\" + expected)
734+
710735
@os_helper.skip_unless_symlink
711736
@unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname')
712737
def test_realpath_symlink_loops(self):

‎Lib/test/test_tarfile.py‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4154,6 +4154,62 @@ def test_parent_symlink2(self):
41544154
self.expect_file('current/')
41554155
self.expect_file('parent/evil')
41564156

4157+
@symlink_test
4158+
def test_parent_symlink3(self):
4159+
# Test a dangling symlink reached through a symlink to a directory
4160+
4161+
with ArchiveMaker() as arc:
4162+
4163+
# `current` links to `.` which is both the destination directory
4164+
# and `current` itself
4165+
arc.add('current', symlink_to='.')
4166+
4167+
# `current/link` is also available as `./link`, and points
4168+
# to a file outside the destination which does not exist yet
4169+
arc.add('current/link', symlink_to='../evil')
4170+
4171+
# Writing through the link creates `../evil`
4172+
arc.add('current/link', content='evil')
4173+
4174+
with self.check_context(arc.open(), 'fully_trusted'):
4175+
if os_helper.can_symlink():
4176+
self.expect_file('current', symlink_to='.')
4177+
self.expect_file('link', symlink_to='../evil')
4178+
self.expect_file('../evil', content='evil')
4179+
else:
4180+
self.expect_file('current/link', content='evil')
4181+
4182+
with self.check_context(arc.open(), 'tar'):
4183+
if os_helper.can_symlink():
4184+
# Fail when extracting a file outside destination
4185+
self.expect_exception(
4186+
tarfile.OutsideDestinationError,
4187+
"'current/link' would be extracted to "
4188+
+ """['"].*evil['"], which is outside """
4189+
+ "the destination")
4190+
else:
4191+
self.expect_file('current/link', content='evil')
4192+
4193+
with self.check_context(arc.open(), 'data'):
4194+
if os_helper.can_symlink():
4195+
if self.dotdot_resolves_early:
4196+
# 'current/../evil' normalises to 'evil', so the link is
4197+
# created; fail when extracting a file through it
4198+
self.expect_exception(
4199+
tarfile.OutsideDestinationError,
4200+
"'current/link' would be extracted to "
4201+
+ """['"].*evil['"], which is outside """
4202+
+ "the destination")
4203+
else:
4204+
# Fail as soon as we have a symlink outside the destination
4205+
self.expect_exception(
4206+
tarfile.LinkOutsideDestinationError,
4207+
"'current/link' would link to "
4208+
+ """['"].*evil['"], which is outside """
4209+
+ "the destination")
4210+
else:
4211+
self.expect_file('current/link', content='evil')
4212+
41574213
@symlink_test
41584214
def test_absolute_symlink(self):
41594215
# Test symlink to an absolute path
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix :func:`os.path.realpath` on Windows returning the wrong path for a broken
2+
relative symbolic link reached through a symbolic link to a directory. This
3+
allowed :mod:`tarfile` extraction with the ``'data'`` or ``'tar'`` filter to
4+
create a file outside the destination directory.

0 commit comments

Comments
 (0)