From 5dbdc22572b4207d7a86676b67b9eeda11add97b Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Thu, 22 Oct 2026 08:08:40 +0000 Subject: [PATCH] gh-160506: Fix ntpath.realpath() for broken links via directory links 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 --- Lib/ntpath.py | 11 +++- Lib/test/test_ntpath.py | 33 +++++++++-- Lib/test/test_tarfile.py | 56 +++++++++++++++++++ ...-10-22-08-08-23.gh-issue-160506.rN4dLq.rst | 4 ++ 4 files changed, 99 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Security/2026-10-22-08-08-23.gh-issue-160506.rN4dLq.rst diff --git a/Lib/ntpath.py b/Lib/ntpath.py index e9e9c4708ee1f1a..0aa6cb7c52f2c69 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -594,7 +594,16 @@ def _readlink_deep(path, ignored_error=OSError): if not islink(old_path): path = old_path break - path = normpath(join(dirname(old_path), path)) + # Resolve the directory holding the link first, so that + # '..' in the target is not applied to a directory link + # in old_path. + link_dir = dirname(old_path) + try: + link_dir = _getfinalpathname(link_dir) + except ignored_error as ex: + if ex.winerror not in allowed_winerror: + raise + path = normpath(join(link_dir, path)) except ignored_error as ex: if ex.winerror in allowed_winerror: break diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index a4de2c78f0bf0c2..96fce534ba75d3e 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -701,9 +701,9 @@ def test_realpath_broken_symlinks(self): self.assertPathEqual(ntpath.realpath(r"broken1\baz"), ABSTFN + r"\broken\bar\baz") self.assertPathEqual(ntpath.realpath("broken2"), - ABSTFN + r"\self\self\missing") + ABSTFN + r"\missing") self.assertPathEqual(ntpath.realpath("broken3"), - ABSTFN + r"\subdir\parent\subdir\parent\missing") + ABSTFN + r"\missing") self.assertPathEqual(ntpath.realpath("broken4"), ABSTFN + r"\missing") self.assertPathEqual(ntpath.realpath("broken5"), @@ -718,14 +718,39 @@ def test_realpath_broken_symlinks(self): self.assertPathEqual(ntpath.realpath(rb"broken1\baz"), os.fsencode(ABSTFN + r"\broken\bar\baz")) self.assertPathEqual(ntpath.realpath(b"broken2"), - os.fsencode(ABSTFN + r"\self\self\missing")) + os.fsencode(ABSTFN + r"\missing")) self.assertPathEqual(ntpath.realpath(rb"broken3"), - os.fsencode(ABSTFN + r"\subdir\parent\subdir\parent\missing")) + os.fsencode(ABSTFN + r"\missing")) self.assertPathEqual(ntpath.realpath(b"broken4"), os.fsencode(ABSTFN + r"\missing")) self.assertPathEqual(ntpath.realpath(b"broken5"), os.fsencode(ABSTFN + r"\missing")) + @os_helper.skip_unless_symlink + @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') + @_parameterize({}, {'strict': ALL_BUT_LAST}, {'strict': ALLOW_MISSING}) + def test_realpath_broken_symlink_in_linked_dir(self, kwargs): + # A relative target is resolved against the directory containing + # the link, not against the path by which the link was reached. + ABSTFN = ntpath.abspath(os_helper.TESTFN) + os.makedirs(ABSTFN + r"\sub") + self.addCleanup(os_helper.rmtree, ABSTFN) + os.symlink(".", ABSTFN + r"\sub\alias", target_is_directory=True) + os.symlink(r"..\missing", ABSTFN + r"\sub\broken") + os.symlink(r"alias\alias\broken", ABSTFN + r"\sub\broken2") + + expected = ABSTFN + r"\missing" + for name in "broken", r"alias\broken", "broken2", r"alias\broken2": + path = ABSTFN + "\\sub\\" + name + with self.subTest(name=name): + self.assertPathEqual(ntpath.realpath(path, **kwargs), expected) + self.assertPathEqual( + ntpath.realpath(os.fsencode(path), **kwargs), + os.fsencode(expected)) + self.assertPathEqual( + ntpath.realpath("\\\\?\\" + path, **kwargs), + "\\\\?\\" + expected) + @os_helper.skip_unless_symlink @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') def test_realpath_symlink_loops(self): diff --git a/Lib/test/test_tarfile.py b/Lib/test/test_tarfile.py index dba51d7131aa356..ef2de974c92d77c 100644 --- a/Lib/test/test_tarfile.py +++ b/Lib/test/test_tarfile.py @@ -4506,6 +4506,62 @@ def test_parent_symlink2(self): self.expect_file('current/') self.expect_file('parent/evil') + @symlink_test + def test_parent_symlink3(self): + # Test a dangling symlink reached through a symlink to a directory + + with ArchiveMaker() as arc: + + # `current` links to `.` which is both the destination directory + # and `current` itself + arc.add('current', symlink_to='.') + + # `current/link` is also available as `./link`, and points + # to a file outside the destination which does not exist yet + arc.add('current/link', symlink_to='../evil') + + # Writing through the link creates `../evil` + arc.add('current/link', content='evil') + + with self.check_context(arc.open(), 'fully_trusted'): + if os_helper.can_symlink(): + self.expect_file('current', symlink_to='.') + self.expect_file('link', symlink_to='../evil') + self.expect_file('../evil', content='evil') + else: + self.expect_file('current/link', content='evil') + + with self.check_context(arc.open(), 'tar'): + if os_helper.can_symlink(): + # Fail when extracting a file outside destination + self.expect_exception( + tarfile.OutsideDestinationError, + "'current/link' would be extracted to " + + """['"].*evil['"], which is outside """ + + "the destination") + else: + self.expect_file('current/link', content='evil') + + with self.check_context(arc.open(), 'data'): + if os_helper.can_symlink(): + if self.dotdot_resolves_early: + # 'current/../evil' normalises to 'evil', so the link is + # created; fail when extracting a file through it + self.expect_exception( + tarfile.OutsideDestinationError, + "'current/link' would be extracted to " + + """['"].*evil['"], which is outside """ + + "the destination") + else: + # Fail as soon as we have a symlink outside the destination + self.expect_exception( + tarfile.LinkOutsideDestinationError, + "'current/link' would link to " + + """['"].*evil['"], which is outside """ + + "the destination") + else: + self.expect_file('current/link', content='evil') + @symlink_test def test_absolute_symlink(self): # Test symlink to an absolute path diff --git a/Misc/NEWS.d/next/Security/2026-10-22-08-08-23.gh-issue-160506.rN4dLq.rst b/Misc/NEWS.d/next/Security/2026-10-22-08-08-23.gh-issue-160506.rN4dLq.rst new file mode 100644 index 000000000000000..07ba8fdbff2cef4 --- /dev/null +++ b/Misc/NEWS.d/next/Security/2026-10-22-08-08-23.gh-issue-160506.rN4dLq.rst @@ -0,0 +1,4 @@ +Fix :func:`os.path.realpath` on Windows returning the wrong path for a broken +relative symbolic link reached through a symbolic link to a directory. This +allowed :mod:`tarfile` extraction with the ``'data'`` or ``'tar'`` filter to +create a file outside the destination directory.