From 178ba5bd032dbefe838f9792dac905c46e9bb04f Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" <68491+gpshead@users.noreply.github.com> Date: Sat, 31 Oct 2026 12:09:04 -0700 Subject: [PATCH 1/4] 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 65665a4aba51b4718085eae25849dd4a15a4a295) Co-authored-by: Gregory P. Smith <68491+gpshead@users.noreply.github.com> --- 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 ba19b1935803640..b5549b3c1221477 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 e10e062639ff1d6..08350983a037de9 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -681,9 +681,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"), @@ -698,14 +698,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 8002c3032467939..dd9ecebbee5d462 100644 --- a/Lib/test/test_tarfile.py +++ b/Lib/test/test_tarfile.py @@ -4220,6 +4220,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. From 7a5e9348683057ecc14d34357200f49ad7565793 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Mon, 9 Nov 2026 00:18:50 +0000 Subject: [PATCH 2/4] Drop ALL_BUT_LAST from the backported test; it does not exist in 3.14 --- Lib/test/test_ntpath.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index 08350983a037de9..fe148e4cf2a4cf0 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -708,7 +708,7 @@ def test_realpath_broken_symlinks(self): @os_helper.skip_unless_symlink @unittest.skipUnless(HAVE_GETFINALPATHNAME, 'need _getfinalpathname') - @_parameterize({}, {'strict': ALL_BUT_LAST}, {'strict': ALLOW_MISSING}) + @_parameterize({}, {'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. From ebcf30f8dc7822e19d188d1178b6f1f436742dbe Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Mon, 9 Nov 2026 02:16:05 +0000 Subject: [PATCH 3/4] Expect an OSError from test_parent_symlink3 fully_trusted on Windows Before 3.15, tarfile keeps the '/' in symlink targets on Windows (gh-57911), so writing through the '../evil' link fails, as already handled in test_parent_symlink2. --- Lib/test/test_tarfile.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_tarfile.py b/Lib/test/test_tarfile.py index dd9ecebbee5d462..5ab81fde0cb527b 100644 --- a/Lib/test/test_tarfile.py +++ b/Lib/test/test_tarfile.py @@ -4239,9 +4239,14 @@ def test_parent_symlink3(self): 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') + if os.name == 'nt': + # Windows does not accept the '/' in the link target, + # so writing through the link fails + self.expect_exception(OSError) + else: + 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') From 280b30f1ecb9f005e133655a9ef64cab1ae571f4 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Mon, 9 Nov 2026 03:11:36 +0000 Subject: [PATCH 4/4] Skip test_parent_symlink3 on Windows Before 3.15, tarfile keeps the '/' in symlink targets on Windows (gh-57911), so the links it creates cannot be followed and realpath() fails on them. test_ntpath covers the fix on Windows. --- Lib/test/test_tarfile.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_tarfile.py b/Lib/test/test_tarfile.py index 5ab81fde0cb527b..119e4ca354e6bc2 100644 --- a/Lib/test/test_tarfile.py +++ b/Lib/test/test_tarfile.py @@ -4221,6 +4221,9 @@ def test_parent_symlink2(self): self.expect_file('parent/evil') @symlink_test + @unittest.skipIf(os.name == 'nt', + "tarfile keeps '/' in symlink targets on Windows " + "(gh-57911), so the links cannot be followed") def test_parent_symlink3(self): # Test a dangling symlink reached through a symlink to a directory @@ -4239,14 +4242,9 @@ def test_parent_symlink3(self): with self.check_context(arc.open(), 'fully_trusted'): if os_helper.can_symlink(): - if os.name == 'nt': - # Windows does not accept the '/' in the link target, - # so writing through the link fails - self.expect_exception(OSError) - else: - self.expect_file('current', symlink_to='.') - self.expect_file('link', symlink_to='../evil') - self.expect_file('../evil', content='evil') + 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')