Skip to content

Commit 26a55db

Browse files
Merge pull request from GHSA-wm8q-9975-xh5v
* Allow only some image types to be displayed inline. Force download for others, especially SVG images. By default we use a list of allowed types. You can switch a to a list of denied types by setting OS environment variable ``OFS_IMAGE_USE_DENYLIST=1``. This change only affects direct URL access. ``<img src="image.svg" />`` works the same as before. See security advisory: GHSA-wm8q-9975-xh5v * svg download: only encode filename when it is not bytes. On Python 2.7 it is already bytes. * Use guess_extension as first guess for the extension of an image. * Support overriding ALLOWED_INLINE_MIMETYPES and DISALLOWED_INLINE_MIMETYPES. * Test that filename of attachment gets encoded correctly. * Add CVE --------- Co-authored-by: Michael Howitz <icemac@gmx.net>
1 parent a0aa2ff commit 26a55db

File tree

3 files changed

+165
-0
lines changed

3 files changed

+165
-0
lines changed

Diff for: CHANGES.rst

+10
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,16 @@ https://zope.readthedocs.io/en/2.13/CHANGES.html
1010
4.8.10 (unreleased)
1111
-------------------
1212

13+
- Allow only some image types to be displayed inline. Force download for
14+
others, especially SVG images. By default we use a list of allowed types.
15+
You can switch a to a list of denied types by setting OS environment variable
16+
``OFS_IMAGE_USE_DENYLIST=1``. You can override the allowed list with
17+
environment variable ``ALLOWED_INLINE_MIMETYPES`` and the disallowed list
18+
with ``DISALLOWED_INLINE_MIMETYPES``. Separate multiple entries by either
19+
comma or space. This change only affects direct URL access.
20+
``<img src="image.svg" />`` works the same as before. (CVE-2023-42458)
21+
See `security advisory <https://github.com/zopefoundation/Zope/security/advisories/GHSA-wm8q-9975-xh5v>`_.
22+
1323
- Tighten down the ZMI frame source logic to only allow site-local sources.
1424
Problem reported by Miguel Segovia Gil.
1525

Diff for: src/OFS/Image.py

+99
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,19 @@
1313
"""Image object
1414
"""
1515

16+
import os
1617
import struct
1718
from email.generator import _make_boundary
1819
from io import BytesIO
1920
from io import TextIOBase
21+
from mimetypes import guess_extension
2022
from tempfile import TemporaryFile
2123
from warnings import warn
2224

2325
from six import PY2
2426
from six import binary_type
2527
from six import text_type
28+
from six.moves.urllib.parse import quote
2629

2730
import ZPublisher.HTTPRequest
2831
from AccessControl.class_init import InitializeClass
@@ -61,6 +64,64 @@
6164
from cgi import escape
6265

6366

67+
def _get_list_from_env(name, default=None):
68+
"""Get list from environment variable.
69+
70+
Supports splitting on comma or white space.
71+
Use the default as fallback only when the variable is not set.
72+
So if the env variable is set to an empty string, this will ignore the
73+
default and return an empty list.
74+
"""
75+
value = os.environ.get(name)
76+
if value is None:
77+
return default or []
78+
value = value.strip()
79+
if "," in value:
80+
return value.split(",")
81+
return value.split()
82+
83+
84+
# We have one list for allowed, and one for disallowed inline mimetypes.
85+
# This is for security purposes.
86+
# By default we use the allowlist. We give integrators the option to choose
87+
# the denylist via an environment variable.
88+
ALLOWED_INLINE_MIMETYPES = _get_list_from_env(
89+
"ALLOWED_INLINE_MIMETYPES",
90+
default=[
91+
"image/gif",
92+
# The mimetypes registry lists several for jpeg 2000:
93+
"image/jp2",
94+
"image/jpeg",
95+
"image/jpeg2000-image",
96+
"image/jpeg2000",
97+
"image/jpx",
98+
"image/png",
99+
"image/webp",
100+
"image/x-icon",
101+
"image/x-jpeg2000-image",
102+
"text/plain",
103+
# By popular request we allow PDF:
104+
"application/pdf",
105+
]
106+
)
107+
DISALLOWED_INLINE_MIMETYPES = _get_list_from_env(
108+
"DISALLOWED_INLINE_MIMETYPES",
109+
default=[
110+
"application/javascript",
111+
"application/x-javascript",
112+
"text/javascript",
113+
"text/html",
114+
"image/svg+xml",
115+
"image/svg+xml-compressed",
116+
]
117+
)
118+
try:
119+
USE_DENYLIST = os.environ.get("OFS_IMAGE_USE_DENYLIST")
120+
USE_DENYLIST = bool(int(USE_DENYLIST))
121+
except (ValueError, TypeError, AttributeError):
122+
USE_DENYLIST = False
123+
124+
64125
manage_addFileForm = DTMLFile(
65126
'dtml/imageAdd',
66127
globals(),
@@ -120,6 +181,13 @@ class File(
120181
Cacheable
121182
):
122183
"""A File object is a content object for arbitrary files."""
184+
# You can control which mimetypes may be shown inline
185+
# and which must always be downloaded, for security reasons.
186+
# Make the configuration available on the class.
187+
# Then subclasses can override this.
188+
allowed_inline_mimetypes = ALLOWED_INLINE_MIMETYPES
189+
disallowed_inline_mimetypes = DISALLOWED_INLINE_MIMETYPES
190+
use_denylist = USE_DENYLIST
123191

124192
meta_type = 'File'
125193
zmi_icon = 'far fa-file-archive'
@@ -418,6 +486,19 @@ def _range_request_handler(self, REQUEST, RESPONSE):
418486
b'\r\n--' + boundary.encode('ascii') + b'--\r\n')
419487
return True
420488

489+
def _should_force_download(self):
490+
# If this returns True, the caller should set a
491+
# Content-Disposition header with filename.
492+
mimetype = self.content_type
493+
if not mimetype:
494+
return False
495+
if self.use_denylist:
496+
# We explicitly deny a few mimetypes, and allow the rest.
497+
return mimetype in self.disallowed_inline_mimetypes
498+
# Use the allowlist.
499+
# We only explicitly allow a few mimetypes, and deny the rest.
500+
return mimetype not in self.allowed_inline_mimetypes
501+
421502
@security.protected(View)
422503
def index_html(self, REQUEST, RESPONSE):
423504
"""
@@ -456,6 +537,24 @@ def index_html(self, REQUEST, RESPONSE):
456537
RESPONSE.setHeader('Content-Length', self.size)
457538
RESPONSE.setHeader('Accept-Ranges', 'bytes')
458539

540+
if self._should_force_download():
541+
# We need a filename, even a dummy one if needed.
542+
filename = self.getId()
543+
if "." not in filename:
544+
# This either returns None or ".some_extension"
545+
ext = guess_extension(self.content_type, strict=False)
546+
if not ext:
547+
# image/svg+xml -> svg
548+
ext = "." + self.content_type.split("/")[-1].split("+")[0]
549+
filename += ext
550+
if not isinstance(filename, bytes):
551+
filename = filename.encode("utf8")
552+
filename = quote(filename)
553+
RESPONSE.setHeader(
554+
"Content-Disposition",
555+
"attachment; filename*=UTF-8''{}".format(filename),
556+
)
557+
459558
if self.ZCacheable_isCachingEnabled():
460559
result = self.ZCacheable_get(default=None)
461560
if result is not None:

Diff for: src/OFS/tests/testFileAndImage.py

+56
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,7 @@ def testViewImageOrFile(self):
368368
response = request.RESPONSE
369369
result = self.file.index_html(request, response)
370370
self.assertEqual(result, self.data)
371+
self.assertIsNone(response.getHeader("Content-Disposition"))
371372

372373
def test_interfaces(self):
373374
from OFS.Image import Image
@@ -382,6 +383,61 @@ def test_text_representation_is_tag(self):
382383
' alt="" title="" height="16" width="16" />')
383384

384385

386+
class SVGTests(ImageTests):
387+
content_type = 'image/svg+xml'
388+
389+
def testViewImageOrFile(self):
390+
request = self.app.REQUEST
391+
response = request.RESPONSE
392+
result = self.file.index_html(request, response)
393+
self.assertEqual(result, self.data)
394+
self.assertEqual(
395+
response.getHeader("Content-Disposition"),
396+
"attachment; filename*=UTF-8''file.svg",
397+
)
398+
399+
def testViewImageOrFileNonAscii(self):
400+
try:
401+
factory = getattr(self.app, self.factory)
402+
factory('hällo',
403+
file=self.data, content_type=self.content_type)
404+
transaction.commit()
405+
except Exception:
406+
transaction.abort()
407+
self.connection.close()
408+
raise
409+
transaction.begin()
410+
image = getattr(self.app, 'hällo')
411+
request = self.app.REQUEST
412+
response = request.RESPONSE
413+
result = image.index_html(request, response)
414+
self.assertEqual(result, self.data)
415+
self.assertEqual(
416+
response.getHeader("Content-Disposition"),
417+
"attachment; filename*=UTF-8''h%C3%A4llo.svg",
418+
)
419+
420+
def testViewImageOrFile_with_denylist(self):
421+
request = self.app.REQUEST
422+
response = request.RESPONSE
423+
self.file.use_denylist = True
424+
result = self.file.index_html(request, response)
425+
self.assertEqual(result, self.data)
426+
self.assertEqual(
427+
response.getHeader("Content-Disposition"),
428+
"attachment; filename*=UTF-8''file.svg",
429+
)
430+
431+
def testViewImageOrFile_with_empty_denylist(self):
432+
request = self.app.REQUEST
433+
response = request.RESPONSE
434+
self.file.use_denylist = True
435+
self.file.disallowed_inline_mimetypes = []
436+
result = self.file.index_html(request, response)
437+
self.assertEqual(result, self.data)
438+
self.assertIsNone(response.getHeader("Content-Disposition"))
439+
440+
385441
class FileEditTests(Testing.ZopeTestCase.FunctionalTestCase):
386442
"""Browser testing ..Image.File"""
387443

0 commit comments

Comments
 (0)