Skip to content

Commit 18e07bf

Browse files
[3.15] gh-160506: Fix ntpath.realpath() for broken links via directory links (GH-160507) (#161026)
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 f254c47 commit 18e07bf

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
@@ -691,9 +691,9 @@ def test_realpath_broken_symlinks(self):
691691
self.assertPathEqual(ntpath.realpath(r"broken1\baz"),
692692
ABSTFN + r"\broken\bar\baz")
693693
self.assertPathEqual(ntpath.realpath("broken2"),
694-
ABSTFN + r"\self\self\missing")
694+
ABSTFN + r"\missing")
695695
self.assertPathEqual(ntpath.realpath("broken3"),
696-
ABSTFN + r"\subdir\parent\subdir\parent\missing")
696+
ABSTFN + r"\missing")
697697
self.assertPathEqual(ntpath.realpath("broken4"),
698698
ABSTFN + r"\missing")
699699
self.assertPathEqual(ntpath.realpath("broken5"),
@@ -708,14 +708,39 @@ def test_realpath_broken_symlinks(self):
708708
self.assertPathEqual(ntpath.realpath(rb"broken1\baz"),
709709
os.fsencode(ABSTFN + r"\broken\bar\baz"))
710710
self.assertPathEqual(ntpath.realpath(b"broken2"),
711-
os.fsencode(ABSTFN + r"\self\self\missing"))
711+
os.fsencode(ABSTFN + r"\missing"))
712712
self.assertPathEqual(ntpath.realpath(rb"broken3"),
713-
os.fsencode(ABSTFN + r"\subdir\parent\subdir\parent\missing"))
713+
os.fsencode(ABSTFN + r"\missing"))
714714
self.assertPathEqual(ntpath.realpath(b"broken4"),
715715
os.fsencode(ABSTFN + r"\missing"))
716716
self.assertPathEqual(ntpath.realpath(b"broken5"),
717717
os.fsencode(ABSTFN + r"\missing"))
718718

719+
@os_helper.skip_unless_symlink
720+
@unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname')
721+
@_parameterize({}, {'strict': ALL_BUT_LAST}, {'strict': ALLOW_MISSING})
722+
def test_realpath_broken_symlink_in_linked_dir(self, kwargs):
723+
# A relative target is resolved against the directory containing
724+
# the link, not against the path by which the link was reached.
725+
ABSTFN = ntpath.abspath(os_helper.TESTFN)
726+
os.makedirs(ABSTFN + r"\sub")
727+
self.addCleanup(os_helper.rmtree, ABSTFN)
728+
os.symlink(".", ABSTFN + r"\sub\alias", target_is_directory=True)
729+
os.symlink(r"..\missing", ABSTFN + r"\sub\broken")
730+
os.symlink(r"alias\alias\broken", ABSTFN + r"\sub\broken2")
731+
732+
expected = ABSTFN + r"\missing"
733+
for name in "broken", r"alias\broken", "broken2", r"alias\broken2":
734+
path = ABSTFN + "\\sub\\" + name
735+
with self.subTest(name=name):
736+
self.assertPathEqual(ntpath.realpath(path, **kwargs), expected)
737+
self.assertPathEqual(
738+
ntpath.realpath(os.fsencode(path), **kwargs),
739+
os.fsencode(expected))
740+
self.assertPathEqual(
741+
ntpath.realpath("\\\\?\\" + path, **kwargs),
742+
"\\\\?\\" + expected)
743+
719744
@os_helper.skip_unless_symlink
720745
@unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname')
721746
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
@@ -4279,6 +4279,62 @@ def test_parent_symlink2(self):
42794279
self.expect_file('current/')
42804280
self.expect_file('parent/evil')
42814281

4282+
@symlink_test
4283+
def test_parent_symlink3(self):
4284+
# Test a dangling symlink reached through a symlink to a directory
4285+
4286+
with ArchiveMaker() as arc:
4287+
4288+
# `current` links to `.` which is both the destination directory
4289+
# and `current` itself
4290+
arc.add('current', symlink_to='.')
4291+
4292+
# `current/link` is also available as `./link`, and points
4293+
# to a file outside the destination which does not exist yet
4294+
arc.add('current/link', symlink_to='../evil')
4295+
4296+
# Writing through the link creates `../evil`
4297+
arc.add('current/link', content='evil')
4298+
4299+
with self.check_context(arc.open(), 'fully_trusted'):
4300+
if os_helper.can_symlink():
4301+
self.expect_file('current', symlink_to='.')
4302+
self.expect_file('link', symlink_to='../evil')
4303+
self.expect_file('../evil', content='evil')
4304+
else:
4305+
self.expect_file('current/link', content='evil')
4306+
4307+
with self.check_context(arc.open(), 'tar'):
4308+
if os_helper.can_symlink():
4309+
# Fail when extracting a file outside destination
4310+
self.expect_exception(
4311+
tarfile.OutsideDestinationError,
4312+
"'current/link' would be extracted to "
4313+
+ """['"].*evil['"], which is outside """
4314+
+ "the destination")
4315+
else:
4316+
self.expect_file('current/link', content='evil')
4317+
4318+
with self.check_context(arc.open(), 'data'):
4319+
if os_helper.can_symlink():
4320+
if self.dotdot_resolves_early:
4321+
# 'current/../evil' normalises to 'evil', so the link is
4322+
# created; fail when extracting a file through it
4323+
self.expect_exception(
4324+
tarfile.OutsideDestinationError,
4325+
"'current/link' would be extracted to "
4326+
+ """['"].*evil['"], which is outside """
4327+
+ "the destination")
4328+
else:
4329+
# Fail as soon as we have a symlink outside the destination
4330+
self.expect_exception(
4331+
tarfile.LinkOutsideDestinationError,
4332+
"'current/link' would link to "
4333+
+ """['"].*evil['"], which is outside """
4334+
+ "the destination")
4335+
else:
4336+
self.expect_file('current/link', content='evil')
4337+
42824338
@symlink_test
42834339
def test_absolute_symlink(self):
42844340
# 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)