Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion Lib/ntpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 29 additions & 4 deletions Lib/test/test_ntpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand All @@ -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):
Expand Down
56 changes: 56 additions & 0 deletions Lib/test/test_tarfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Loading