Skip to content

Commit 7f3cce3

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 3d6d2a6 commit 7f3cce3

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
@@ -601,7 +601,16 @@ def _readlink_deep(path, ignored_error=OSError):
601601
if not islink(old_path):
602602
path = old_path
603603
break
604-
path = normpath(join(dirname(old_path), path))
604+
# Resolve the directory holding the link first, so that
605+
# '..' in the target is not applied to a directory link
606+
# in old_path.
607+
link_dir = dirname(old_path)
608+
try:
609+
link_dir = _getfinalpathname(link_dir)
610+
except ignored_error as ex:
611+
if ex.winerror not in allowed_winerror:
612+
raise
613+
path = normpath(join(link_dir, path))
605614
except ignored_error as ex:
606615
if ex.winerror in allowed_winerror:
607616
break

‎Lib/test/test_ntpath.py‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -587,9 +587,9 @@ def test_realpath_broken_symlinks(self):
587587
self.assertPathEqual(ntpath.realpath(r"broken1\baz"),
588588
ABSTFN + r"\broken\bar\baz")
589589
self.assertPathEqual(ntpath.realpath("broken2"),
590-
ABSTFN + r"\self\self\missing")
590+
ABSTFN + r"\missing")
591591
self.assertPathEqual(ntpath.realpath("broken3"),
592-
ABSTFN + r"\subdir\parent\subdir\parent\missing")
592+
ABSTFN + r"\missing")
593593
self.assertPathEqual(ntpath.realpath("broken4"),
594594
ABSTFN + r"\missing")
595595
self.assertPathEqual(ntpath.realpath("broken5"),
@@ -604,14 +604,39 @@ def test_realpath_broken_symlinks(self):
604604
self.assertPathEqual(ntpath.realpath(rb"broken1\baz"),
605605
os.fsencode(ABSTFN + r"\broken\bar\baz"))
606606
self.assertPathEqual(ntpath.realpath(b"broken2"),
607-
os.fsencode(ABSTFN + r"\self\self\missing"))
607+
os.fsencode(ABSTFN + r"\missing"))
608608
self.assertPathEqual(ntpath.realpath(rb"broken3"),
609-
os.fsencode(ABSTFN + r"\subdir\parent\subdir\parent\missing"))
609+
os.fsencode(ABSTFN + r"\missing"))
610610
self.assertPathEqual(ntpath.realpath(b"broken4"),
611611
os.fsencode(ABSTFN + r"\missing"))
612612
self.assertPathEqual(ntpath.realpath(b"broken5"),
613613
os.fsencode(ABSTFN + r"\missing"))
614614

615+
@os_helper.skip_unless_symlink
616+
@unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname')
617+
@_parameterize({}, {'strict': ALL_BUT_LAST}, {'strict': ALLOW_MISSING})
618+
def test_realpath_broken_symlink_in_linked_dir(self, kwargs):
619+
# A relative target is resolved against the directory containing
620+
# the link, not against the path by which the link was reached.
621+
ABSTFN = ntpath.abspath(os_helper.TESTFN)
622+
os.makedirs(ABSTFN + r"\sub")
623+
self.addCleanup(os_helper.rmtree, ABSTFN)
624+
os.symlink(".", ABSTFN + r"\sub\alias", target_is_directory=True)
625+
os.symlink(r"..\missing", ABSTFN + r"\sub\broken")
626+
os.symlink(r"alias\alias\broken", ABSTFN + r"\sub\broken2")
627+
628+
expected = ABSTFN + r"\missing"
629+
for name in "broken", r"alias\broken", "broken2", r"alias\broken2":
630+
path = ABSTFN + "\\sub\\" + name
631+
with self.subTest(name=name):
632+
self.assertPathEqual(ntpath.realpath(path, **kwargs), expected)
633+
self.assertPathEqual(
634+
ntpath.realpath(os.fsencode(path), **kwargs),
635+
os.fsencode(expected))
636+
self.assertPathEqual(
637+
ntpath.realpath("\\\\?\\" + path, **kwargs),
638+
"\\\\?\\" + expected)
639+
615640
@os_helper.skip_unless_symlink
616641
@unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname')
617642
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
@@ -3922,6 +3922,62 @@ def test_parent_symlink2(self):
39223922
self.expect_file('current/')
39233923
self.expect_file('parent/evil')
39243924

3925+
@symlink_test
3926+
def test_parent_symlink3(self):
3927+
# Test a dangling symlink reached through a symlink to a directory
3928+
3929+
with ArchiveMaker() as arc:
3930+
3931+
# `current` links to `.` which is both the destination directory
3932+
# and `current` itself
3933+
arc.add('current', symlink_to='.')
3934+
3935+
# `current/link` is also available as `./link`, and points
3936+
# to a file outside the destination which does not exist yet
3937+
arc.add('current/link', symlink_to='../evil')
3938+
3939+
# Writing through the link creates `../evil`
3940+
arc.add('current/link', content='evil')
3941+
3942+
with self.check_context(arc.open(), 'fully_trusted'):
3943+
if os_helper.can_symlink():
3944+
self.expect_file('current', symlink_to='.')
3945+
self.expect_file('link', symlink_to='../evil')
3946+
self.expect_file('../evil', content='evil')
3947+
else:
3948+
self.expect_file('current/link', content='evil')
3949+
3950+
with self.check_context(arc.open(), 'tar'):
3951+
if os_helper.can_symlink():
3952+
# Fail when extracting a file outside destination
3953+
self.expect_exception(
3954+
tarfile.OutsideDestinationError,
3955+
"'current/link' would be extracted to "
3956+
+ """['"].*evil['"], which is outside """
3957+
+ "the destination")
3958+
else:
3959+
self.expect_file('current/link', content='evil')
3960+
3961+
with self.check_context(arc.open(), 'data'):
3962+
if os_helper.can_symlink():
3963+
if self.dotdot_resolves_early:
3964+
# 'current/../evil' normalises to 'evil', so the link is
3965+
# created; fail when extracting a file through it
3966+
self.expect_exception(
3967+
tarfile.OutsideDestinationError,
3968+
"'current/link' would be extracted to "
3969+
+ """['"].*evil['"], which is outside """
3970+
+ "the destination")
3971+
else:
3972+
# Fail as soon as we have a symlink outside the destination
3973+
self.expect_exception(
3974+
tarfile.LinkOutsideDestinationError,
3975+
"'current/link' would link to "
3976+
+ """['"].*evil['"], which is outside """
3977+
+ "the destination")
3978+
else:
3979+
self.expect_file('current/link', content='evil')
3980+
39253981
@symlink_test
39263982
def test_absolute_symlink(self):
39273983
# 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)