GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-123270: Replaced SanitizedNames with a more surgical fix. by jaraco · Pull Request #123354 · python/cpython

gh-123270: Replaced SanitizedNames with a more surgical fix. #123354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jaraco merged 1 commit into python:main from jaraco:gh-123270/secure-allowed-names
Aug 27, 2024

Uh oh!

There was an error while loading. Please reload this page.
Merged

gh-123270: Replaced SanitizedNames with a more surgical fix. #123354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
55c2795
gh-123270: Replaced SanitizedNames with a more surgical fix.
jaraco Aug 26, 2024
File filter
Filter by extension
Clear filters
Conversations
Failed to load comments.
Loading

Uh oh!

There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading

Uh oh!

There was an error while loading. Please reload this page.
Diff view
Diff view

There are no files selected for viewing

73 changes: 67 additions & 6 deletions 73 Lib/test/test_zipfile/_path/test_path.py
Show comments
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import pickle
import stat
import sys
import time
import unittest
import zipfile
import zipfile._path
Expand Down Expand Up @@ -592,7 +593,11 @@ def test_getinfo_missing(self, alpharep):

def test_malformed_paths(self):
"""
Path should handle malformed paths.
Path should handle malformed paths gracefully.

Paths with leading slashes are not visible.

Paths with dots are treated like regular files.
"""
data = io.BytesIO()
zf = zipfile.ZipFile(data, "w")
Expand All @@ -601,15 +606,71 @@ def test_malformed_paths(self):
zf.writestr("../parent.txt", b"content")
zf.filename = ''
root = zipfile.Path(zf)
assert list(map(str, root.iterdir())) == [
'one-slash.txt',
'two-slash.txt',
'parent.txt',
]
assert list(map(str, root.iterdir())) == ['../']
assert root.joinpath('..').joinpath('parent.txt').read_bytes() == b'content'

def test_unsupported_names(self):
"""
Path segments with special characters are readable.

On some platforms or file systems, characters like
``:`` and ``?`` are not allowed, but they are valid
in the zip file.
"""
data = io.BytesIO()
zf = zipfile.ZipFile(data, "w")
zf.writestr("path?", b"content")
zf.writestr("V: NMS.flac", b"fLaC...")
zf.filename = ''
root = zipfile.Path(zf)
contents = root.iterdir()
assert next(contents).name == 'path?'
assert next(contents).name == 'V: NMS.flac'
assert root.joinpath('V: NMS.flac').read_bytes() == b"fLaC..."

def test_backslash_not_separator(self):
"""
In a zip file, backslashes are not separators.
"""
data = io.BytesIO()
zf = zipfile.ZipFile(data, "w")
zf.writestr(DirtyZipInfo.for_name("foo\\bar", zf), b"content")
zf.filename = ''
root = zipfile.Path(zf)
(first,) = root.iterdir()
assert not first.is_dir()
assert first.name == 'foo\\bar'

@pass_alpharep
def test_interface(self, alpharep):
from importlib.resources.abc import Traversable

zf = zipfile.Path(alpharep)
assert isinstance(zf, Traversable)


class DirtyZipInfo(zipfile.ZipInfo):
"""
Bypass name sanitization.
"""

def __init__(self, filename, *args, **kwargs):
super().__init__(filename, *args, **kwargs)
self.filename = filename

@classmethod
def for_name(cls, name, archive):
"""
Construct the same way that ZipFile.writestr does.

TODO: extract this functionality and re-use
"""
self = cls(filename=name, date_time=time.localtime(time.time())[:6])
self.compress_type = archive.compression
self.compress_level = archive.compresslevel
if self.filename.endswith('/'): # pragma: no cover
self.external_attr = 0o40775 << 16 # drwxrwxr-x
self.external_attr |= 0x10 # MS-DOS directory flag
else:
self.external_attr = 0o600 << 16 # ?rw-------
return self
82 changes: 17 additions & 65 deletions 82 Lib/zipfile/_path/__init__.py
Show comments
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
"""
A Path-like interface for zipfiles.

This codebase is shared between zipfile.Path in the stdlib
and zipp in PyPI. See
https://github.com/python/importlib_metadata/wiki/Development-Methodology
for more detail.
"""

import io
import posixpath
import zipfile
Expand Down Expand Up @@ -36,7 +45,7 @@ def _parents(path):
def _ancestry(path):
"""
Given a path with elements separated by
posixpath.sep, generate all elements of that path
posixpath.sep, generate all elements of that path.

>>> list(_ancestry('b/d'))
['b/d', 'b']
Expand All @@ -48,9 +57,14 @@ def _ancestry(path):
['b']
>>> list(_ancestry(''))
[]

Multiple separators are treated like a single.

>>> list(_ancestry('//b//d///f//'))
['//b//d///f', '//b//d', '//b']
"""
path = path.rstrip(posixpath.sep)
while path and path != posixpath.sep:
while path.rstrip(posixpath.sep):
yield path
path, tail = posixpath.split(path)

Expand Down Expand Up @@ -85,69 +99,7 @@ def __setstate__(self, state):
super().__init__(*args, **kwargs)


class SanitizedNames:
"""
ZipFile mix-in to ensure names are sanitized.
"""

def namelist(self):
return list(map(self._sanitize, super().namelist()))

@staticmethod
def _sanitize(name):
r"""
Ensure a relative path with posix separators and no dot names.

Modeled after
https://github.com/python/cpython/blob/bcc1be39cb1d04ad9fc0bd1b9193d3972835a57c/Lib/zipfile/__init__.py#L1799-L1813
but provides consistent cross-platform behavior.

>>> san = SanitizedNames._sanitize
>>> san('/foo/bar')
'foo/bar'
>>> san('//foo.txt')
'foo.txt'
>>> san('foo/.././bar.txt')
'foo/bar.txt'
>>> san('foo../.bar.txt')
'foo../.bar.txt'
>>> san('\\foo\\bar.txt')
'foo/bar.txt'
>>> san('D:\\foo.txt')
'D/foo.txt'
>>> san('\\\\server\\share\\file.txt')
'server/share/file.txt'
>>> san('\\\\?\\GLOBALROOT\\Volume3')
'?/GLOBALROOT/Volume3'
>>> san('\\\\.\\PhysicalDrive1\\root')
'PhysicalDrive1/root'

Retain any trailing slash.
>>> san('abc/')
'abc/'

Raises a ValueError if the result is empty.
>>> san('../..')
Traceback (most recent call last):
...
ValueError: Empty filename
"""

def allowed(part):
return part and part not in {'..', '.'}

# Remove the drive letter.
# Don't use ntpath.splitdrive, because that also strips UNC paths
bare = re.sub('^([A-Z]):', r'\1', name, flags=re.IGNORECASE)
clean = bare.replace('\\', '/')
parts = clean.split('/')
joined = '/'.join(filter(allowed, parts))
if not joined:
raise ValueError("Empty filename")
return joined + '/' * name.endswith('/')


class CompleteDirs(InitializedState, SanitizedNames, zipfile.ZipFile):
class CompleteDirs(InitializedState, zipfile.ZipFile):
"""
A ZipFile subclass that ensures that implied directories
are always included in the namelist.
Expand Down
3 changes: 3 additions & 0 deletions 3 Misc/NEWS.d/next/Library/2024-08-26-13-45-20.gh-issue-123270.gXHvNJ.rst
Show comments
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Applied a more surgical fix for malformed payloads in :class:`zipfile.Path`
causing infinite loops (gh-122905) without breaking contents using
legitimate characters.
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.