mirror of
https://github.com/python/cpython.git
synced 2025-10-19 16:03:42 +00:00
gh-57911: Sanitize symlink targets in tarfile on win32 (GH-138309)
This commit is contained in:
Christoph Walcher
GitHub
parent
e76464d161
commit
c1a9c23195
4 changed files with 71 additions and 42 deletions
|
|
@ -463,6 +463,10 @@ tarfile
|
|||
:func:`~tarfile.TarFile.errorlevel` is zero.
|
||||
(Contributed by Matt Prodani and Petr Viktorin in :gh:`112887`
|
||||
and :cve:`2025-4435`.)
|
||||
* :func:`~tarfile.TarFile.extract` and :func:`~tarfile.TarFile.extractall`
|
||||
now replace slashes by backslashes in symlink targets on Windows to prevent
|
||||
creation of corrupted links.
|
||||
(Contributed by Christoph Walcher in :gh:`57911`.)
|
||||
|
||||
|
||||
types
|
||||
|
|
|
|||
|
|
@ -2718,7 +2718,13 @@ def makelink_with_filter(self, tarinfo, targetpath,
|
|||
if os.path.lexists(targetpath):
|
||||
# Avoid FileExistsError on following os.symlink.
|
||||
os.unlink(targetpath)
|
||||
os.symlink(tarinfo.linkname, targetpath)
|
||||
link_target = tarinfo.linkname
|
||||
if os.name == "nt":
|
||||
# gh-57911: Posix-flavoured forward-slash path separators in
|
||||
# symlink targets aren't acknowledged by Windows, resulting
|
||||
# in corrupted links.
|
||||
link_target = link_target.replace("/", os.path.sep)
|
||||
os.symlink(link_target, targetpath)
|
||||
return
|
||||
else:
|
||||
if os.path.exists(tarinfo._link_target):
|
||||
|
|
|
|||
|
|
@ -2777,7 +2777,7 @@ def test_useful_error_message_when_modules_missing(self):
|
|||
str(excinfo.exception),
|
||||
)
|
||||
|
||||
@unittest.skipUnless(os_helper.can_symlink(), 'requires symlink support')
|
||||
@os_helper.skip_unless_symlink
|
||||
@unittest.skipUnless(hasattr(os, 'chmod'), "missing os.chmod")
|
||||
@unittest.mock.patch('os.chmod')
|
||||
def test_deferred_directory_attributes_update(self, mock_chmod):
|
||||
|
|
@ -3663,6 +3663,39 @@ class TestExtractionFilters(unittest.TestCase):
|
|||
# The destination for the extraction, within `outerdir`
|
||||
destdir = outerdir / 'dest'
|
||||
|
||||
@classmethod
|
||||
def setUpClass(cls):
|
||||
# Posix and Windows have different pathname resolution:
|
||||
# either symlink or a '..' component resolve first.
|
||||
# Let's see which we are on.
|
||||
if os_helper.can_symlink():
|
||||
testpath = os.path.join(TEMPDIR, 'resolution_test')
|
||||
os.mkdir(testpath)
|
||||
|
||||
# testpath/current links to `.` which is all of:
|
||||
# - `testpath`
|
||||
# - `testpath/current`
|
||||
# - `testpath/current/current`
|
||||
# - etc.
|
||||
os.symlink('.', os.path.join(testpath, 'current'))
|
||||
|
||||
# we'll test where `testpath/current/../file` ends up
|
||||
with open(os.path.join(testpath, 'current', '..', 'file'), 'w'):
|
||||
pass
|
||||
|
||||
if os.path.exists(os.path.join(testpath, 'file')):
|
||||
# Windows collapses 'current\..' to '.' first, leaving
|
||||
# 'testpath\file'
|
||||
cls.dotdot_resolves_early = True
|
||||
elif os.path.exists(os.path.join(testpath, '..', 'file')):
|
||||
# Posix resolves 'current' to '.' first, leaving
|
||||
# 'testpath/../file'
|
||||
cls.dotdot_resolves_early = False
|
||||
else:
|
||||
raise AssertionError('Could not determine link resolution')
|
||||
else:
|
||||
cls.dotdot_resolves_early = True
|
||||
|
||||
@contextmanager
|
||||
def check_context(self, tar, filter, *, check_flag=True):
|
||||
"""Extracts `tar` to `self.destdir` and allows checking the result
|
||||
|
|
@ -3809,23 +3842,21 @@ def test_parent_symlink(self):
|
|||
arc.add('current', symlink_to='.')
|
||||
|
||||
# effectively points to ./../
|
||||
arc.add('parent', symlink_to='current/..')
|
||||
if self.dotdot_resolves_early:
|
||||
arc.add('parent', symlink_to='current/../..')
|
||||
else:
|
||||
arc.add('parent', symlink_to='current/..')
|
||||
|
||||
arc.add('parent/evil')
|
||||
|
||||
if os_helper.can_symlink():
|
||||
with self.check_context(arc.open(), 'fully_trusted'):
|
||||
if self.raised_exception is not None:
|
||||
# Windows will refuse to create a file that's a symlink to itself
|
||||
# (and tarfile doesn't swallow that exception)
|
||||
self.expect_exception(FileExistsError)
|
||||
# The other cases will fail with this error too.
|
||||
# Skip the rest of this test.
|
||||
return
|
||||
self.expect_file('current', symlink_to='.')
|
||||
if self.dotdot_resolves_early:
|
||||
self.expect_file('parent', symlink_to='current/../..')
|
||||
else:
|
||||
self.expect_file('current', symlink_to='.')
|
||||
self.expect_file('parent', symlink_to='current/..')
|
||||
self.expect_file('../evil')
|
||||
self.expect_file('../evil')
|
||||
|
||||
with self.check_context(arc.open(), 'tar'):
|
||||
self.expect_exception(
|
||||
|
|
@ -3927,35 +3958,6 @@ def test_parent_symlink2(self):
|
|||
# Test interplaying symlinks
|
||||
# Inspired by 'dirsymlink2b' in jwilk/traversal-archives
|
||||
|
||||
# Posix and Windows have different pathname resolution:
|
||||
# either symlink or a '..' component resolve first.
|
||||
# Let's see which we are on.
|
||||
if os_helper.can_symlink():
|
||||
testpath = os.path.join(TEMPDIR, 'resolution_test')
|
||||
os.mkdir(testpath)
|
||||
|
||||
# testpath/current links to `.` which is all of:
|
||||
# - `testpath`
|
||||
# - `testpath/current`
|
||||
# - `testpath/current/current`
|
||||
# - etc.
|
||||
os.symlink('.', os.path.join(testpath, 'current'))
|
||||
|
||||
# we'll test where `testpath/current/../file` ends up
|
||||
with open(os.path.join(testpath, 'current', '..', 'file'), 'w'):
|
||||
pass
|
||||
|
||||
if os.path.exists(os.path.join(testpath, 'file')):
|
||||
# Windows collapses 'current\..' to '.' first, leaving
|
||||
# 'testpath\file'
|
||||
dotdot_resolves_early = True
|
||||
elif os.path.exists(os.path.join(testpath, '..', 'file')):
|
||||
# Posix resolves 'current' to '.' first, leaving
|
||||
# 'testpath/../file'
|
||||
dotdot_resolves_early = False
|
||||
else:
|
||||
raise AssertionError('Could not determine link resolution')
|
||||
|
||||
with ArchiveMaker() as arc:
|
||||
|
||||
# `current` links to `.` which is both the destination directory
|
||||
|
|
@ -3991,7 +3993,7 @@ def test_parent_symlink2(self):
|
|||
|
||||
with self.check_context(arc.open(), 'data'):
|
||||
if os_helper.can_symlink():
|
||||
if dotdot_resolves_early:
|
||||
if self.dotdot_resolves_early:
|
||||
# Fail when extracting a file outside destination
|
||||
self.expect_exception(
|
||||
tarfile.OutsideDestinationError,
|
||||
|
|
@ -4039,6 +4041,21 @@ def test_absolute_symlink(self):
|
|||
tarfile.AbsoluteLinkError,
|
||||
"'parent' is a link to an absolute path")
|
||||
|
||||
@symlink_test
|
||||
@os_helper.skip_unless_symlink
|
||||
def test_symlink_target_seperator_rewrite_on_windows(self):
|
||||
with ArchiveMaker() as arc:
|
||||
arc.add('link', symlink_to="relative/test/path")
|
||||
|
||||
with self.check_context(arc.open(), 'fully_trusted'):
|
||||
self.expect_file('link', type=tarfile.SYMTYPE)
|
||||
link_path = os.path.normpath(self.destdir / "link")
|
||||
link_target = os.readlink(link_path)
|
||||
if os.name == "nt":
|
||||
self.assertEqual(link_target, "relative\\test\\path")
|
||||
else:
|
||||
self.assertEqual(link_target, "relative/test/path")
|
||||
|
||||
def test_absolute_hardlink(self):
|
||||
# Test hardlink to an absolute path
|
||||
# Inspired by 'dirsymlink' in https://github.com/jwilk/traversal-archives
|
||||
|
|
|
|||
|
|
@ -0,0 +1,2 @@
|
|||
When extracting tar files on Windows, slashes in symlink targets will be
|
||||
replaced by backslashes to prevent corrupted links.
|
||||
Loading…
Add table
Add a link
Reference in a new issue