Skip to content

Commit bcb431c

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 ffbb750 commit bcb431c

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
@@ -570,7 +570,16 @@ def _readlink_deep(path, ignored_error=OSError):
570570
if not islink(old_path):
571571
path = old_path
572572
break
573-
path = normpath(join(dirname(old_path), path))
573+
# Resolve the directory holding the link first, so that
574+
# '..' in the target is not applied to a directory link
575+
# in old_path.
576+
link_dir = dirname(old_path)
577+
try:
578+
link_dir = _getfinalpathname(link_dir)
579+
except ignored_error as ex:
580+
if ex.winerror not in allowed_winerror:
581+
raise
582+
path = normpath(join(link_dir, path))
574583
except ignored_error as ex:
575584
if ex.winerror in allowed_winerror:
576585
break

‎Lib/test/test_ntpath.py‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -525,9 +525,9 @@ def test_realpath_broken_symlinks(self):
525525
self.assertPathEqual(ntpath.realpath(r"broken1\baz"),
526526
ABSTFN + r"\broken\bar\baz")
527527
self.assertPathEqual(ntpath.realpath("broken2"),
528-
ABSTFN + r"\self\self\missing")
528+
ABSTFN + r"\missing")
529529
self.assertPathEqual(ntpath.realpath("broken3"),
530-
ABSTFN + r"\subdir\parent\subdir\parent\missing")
530+
ABSTFN + r"\missing")
531531
self.assertPathEqual(ntpath.realpath("broken4"),
532532
ABSTFN + r"\missing")
533533
self.assertPathEqual(ntpath.realpath("broken5"),
@@ -542,14 +542,39 @@ def test_realpath_broken_symlinks(self):
542542
self.assertPathEqual(ntpath.realpath(rb"broken1\baz"),
543543
os.fsencode(ABSTFN + r"\broken\bar\baz"))
544544
self.assertPathEqual(ntpath.realpath(b"broken2"),
545-
os.fsencode(ABSTFN + r"\self\self\missing"))
545+
os.fsencode(ABSTFN + r"\missing"))
546546
self.assertPathEqual(ntpath.realpath(rb"broken3"),
547-
os.fsencode(ABSTFN + r"\subdir\parent\subdir\parent\missing"))
547+
os.fsencode(ABSTFN + r"\missing"))
548548
self.assertPathEqual(ntpath.realpath(b"broken4"),
549549
os.fsencode(ABSTFN + r"\missing"))
550550
self.assertPathEqual(ntpath.realpath(b"broken5"),
551551
os.fsencode(ABSTFN + r"\missing"))
552552

553+
@os_helper.skip_unless_symlink
554+
@unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname')
555+
@_parameterize({}, {'strict': ALL_BUT_LAST}, {'strict': ALLOW_MISSING})
556+
def test_realpath_broken_symlink_in_linked_dir(self, kwargs):
557+
# A relative target is resolved against the directory containing
558+
# the link, not against the path by which the link was reached.
559+
ABSTFN = ntpath.abspath(os_helper.TESTFN)
560+
os.makedirs(ABSTFN + r"\sub")
561+
self.addCleanup(os_helper.rmtree, ABSTFN)
562+
os.symlink(".", ABSTFN + r"\sub\alias", target_is_directory=True)
563+
os.symlink(r"..\missing", ABSTFN + r"\sub\broken")
564+
os.symlink(r"alias\alias\broken", ABSTFN + r"\sub\broken2")
565+
566+
expected = ABSTFN + r"\missing"
567+
for name in "broken", r"alias\broken", "broken2", r"alias\broken2":
568+
path = ABSTFN + "\\sub\\" + name
569+
with self.subTest(name=name):
570+
self.assertPathEqual(ntpath.realpath(path, **kwargs), expected)
571+
self.assertPathEqual(
572+
ntpath.realpath(os.fsencode(path), **kwargs),
573+
os.fsencode(expected))
574+
self.assertPathEqual(
575+
ntpath.realpath("\\\\?\\" + path, **kwargs),
576+
"\\\\?\\" + expected)
577+
553578
@os_helper.skip_unless_symlink
554579
@unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname')
555580
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
@@ -3821,6 +3821,62 @@ def test_parent_symlink2(self):
38213821
self.expect_file('current/')
38223822
self.expect_file('parent/evil')
38233823

3824+
@symlink_test
3825+
def test_parent_symlink3(self):
3826+
# Test a dangling symlink reached through a symlink to a directory
3827+
3828+
with ArchiveMaker() as arc:
3829+
3830+
# `current` links to `.` which is both the destination directory
3831+
# and `current` itself
3832+
arc.add('current', symlink_to='.')
3833+
3834+
# `current/link` is also available as `./link`, and points
3835+
# to a file outside the destination which does not exist yet
3836+
arc.add('current/link', symlink_to='../evil')
3837+
3838+
# Writing through the link creates `../evil`
3839+
arc.add('current/link', content='evil')
3840+
3841+
with self.check_context(arc.open(), 'fully_trusted'):
3842+
if os_helper.can_symlink():
3843+
self.expect_file('current', symlink_to='.')
3844+
self.expect_file('link', symlink_to='../evil')
3845+
self.expect_file('../evil', content='evil')
3846+
else:
3847+
self.expect_file('current/link', content='evil')
3848+
3849+
with self.check_context(arc.open(), 'tar'):
3850+
if os_helper.can_symlink():
3851+
# Fail when extracting a file outside destination
3852+
self.expect_exception(
3853+
tarfile.OutsideDestinationError,
3854+
"'current/link' would be extracted to "
3855+
+ """['"].*evil['"], which is outside """
3856+
+ "the destination")
3857+
else:
3858+
self.expect_file('current/link', content='evil')
3859+
3860+
with self.check_context(arc.open(), 'data'):
3861+
if os_helper.can_symlink():
3862+
if self.dotdot_resolves_early:
3863+
# 'current/../evil' normalises to 'evil', so the link is
3864+
# created; fail when extracting a file through it
3865+
self.expect_exception(
3866+
tarfile.OutsideDestinationError,
3867+
"'current/link' would be extracted to "
3868+
+ """['"].*evil['"], which is outside """
3869+
+ "the destination")
3870+
else:
3871+
# Fail as soon as we have a symlink outside the destination
3872+
self.expect_exception(
3873+
tarfile.LinkOutsideDestinationError,
3874+
"'current/link' would link to "
3875+
+ """['"].*evil['"], which is outside """
3876+
+ "the destination")
3877+
else:
3878+
self.expect_file('current/link', content='evil')
3879+
38243880
@symlink_test
38253881
def test_absolute_symlink(self):
38263882
# 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)