Skip to content

Commit 603b0a1

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 * 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 05f81d9 commit 603b0a1

File tree

3 files changed

+163
-0
lines changed

3 files changed

+163
-0
lines changed

Diff for: CHANGES.rst

+10
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,16 @@ https://github.com/zopefoundation/Zope/blob/4.x/CHANGES.rst
1111
5.8.5 (unreleased)
1212
------------------
1313

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

Diff for: src/OFS/Image.py

+97
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,12 @@
1414
"""
1515

1616
import html
17+
import os
1718
import struct
1819
from email.generator import _make_boundary
1920
from io import BytesIO
21+
from mimetypes import guess_extension
22+
from urllib.parse import quote
2023
from xml.dom import minidom
2124

2225
import ZPublisher.HTTPRequest
@@ -48,6 +51,64 @@
4851
from ZPublisher.HTTPRequest import FileUpload
4952

5053

54+
def _get_list_from_env(name, default=None):
55+
"""Get list from environment variable.
56+
57+
Supports splitting on comma or white space.
58+
Use the default as fallback only when the variable is not set.
59+
So if the env variable is set to an empty string, this will ignore the
60+
default and return an empty list.
61+
"""
62+
value = os.environ.get(name)
63+
if value is None:
64+
return default or []
65+
value = value.strip()
66+
if "," in value:
67+
return value.split(",")
68+
return value.split()
69+
70+
71+
# We have one list for allowed, and one for disallowed inline mimetypes.
72+
# This is for security purposes.
73+
# By default we use the allowlist. We give integrators the option to choose
74+
# the denylist via an environment variable.
75+
ALLOWED_INLINE_MIMETYPES = _get_list_from_env(
76+
"ALLOWED_INLINE_MIMETYPES",
77+
default=[
78+
"image/gif",
79+
# The mimetypes registry lists several for jpeg 2000:
80+
"image/jp2",
81+
"image/jpeg",
82+
"image/jpeg2000-image",
83+
"image/jpeg2000",
84+
"image/jpx",
85+
"image/png",
86+
"image/webp",
87+
"image/x-icon",
88+
"image/x-jpeg2000-image",
89+
"text/plain",
90+
# By popular request we allow PDF:
91+
"application/pdf",
92+
]
93+
)
94+
DISALLOWED_INLINE_MIMETYPES = _get_list_from_env(
95+
"DISALLOWED_INLINE_MIMETYPES",
96+
default=[
97+
"application/javascript",
98+
"application/x-javascript",
99+
"text/javascript",
100+
"text/html",
101+
"image/svg+xml",
102+
"image/svg+xml-compressed",
103+
]
104+
)
105+
try:
106+
USE_DENYLIST = os.environ.get("OFS_IMAGE_USE_DENYLIST")
107+
USE_DENYLIST = bool(int(USE_DENYLIST))
108+
except (ValueError, TypeError, AttributeError):
109+
USE_DENYLIST = False
110+
111+
51112
manage_addFileForm = DTMLFile(
52113
'dtml/imageAdd',
53114
globals(),
@@ -107,6 +168,13 @@ class File(
107168
Cacheable
108169
):
109170
"""A File object is a content object for arbitrary files."""
171+
# You can control which mimetypes may be shown inline
172+
# and which must always be downloaded, for security reasons.
173+
# Make the configuration available on the class.
174+
# Then subclasses can override this.
175+
allowed_inline_mimetypes = ALLOWED_INLINE_MIMETYPES
176+
disallowed_inline_mimetypes = DISALLOWED_INLINE_MIMETYPES
177+
use_denylist = USE_DENYLIST
110178

111179
meta_type = 'File'
112180
zmi_icon = 'far fa-file-archive'
@@ -403,6 +471,19 @@ def _range_request_handler(self, REQUEST, RESPONSE):
403471
b'\r\n--' + boundary.encode('ascii') + b'--\r\n')
404472
return True
405473

474+
def _should_force_download(self):
475+
# If this returns True, the caller should set a
476+
# Content-Disposition header with filename.
477+
mimetype = self.content_type
478+
if not mimetype:
479+
return False
480+
if self.use_denylist:
481+
# We explicitly deny a few mimetypes, and allow the rest.
482+
return mimetype in self.disallowed_inline_mimetypes
483+
# Use the allowlist.
484+
# We only explicitly allow a few mimetypes, and deny the rest.
485+
return mimetype not in self.allowed_inline_mimetypes
486+
406487
@security.protected(View)
407488
def index_html(self, REQUEST, RESPONSE):
408489
"""
@@ -441,6 +522,22 @@ def index_html(self, REQUEST, RESPONSE):
441522
RESPONSE.setHeader('Content-Length', self.size)
442523
RESPONSE.setHeader('Accept-Ranges', 'bytes')
443524

525+
if self._should_force_download():
526+
# We need a filename, even a dummy one if needed.
527+
filename = self.getId()
528+
if "." not in filename:
529+
# This either returns None or ".some_extension"
530+
ext = guess_extension(self.content_type, strict=False)
531+
if not ext:
532+
# image/svg+xml -> svg
533+
ext = "." + self.content_type.split("/")[-1].split("+")[0]
534+
filename += f"{ext}"
535+
filename = quote(filename.encode("utf8"))
536+
RESPONSE.setHeader(
537+
"Content-Disposition",
538+
f"attachment; filename*=UTF-8''{filename}",
539+
)
540+
444541
if self.ZCacheable_isCachingEnabled():
445542
result = self.ZCacheable_get(default=None)
446543
if result is not None:

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

+56
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,7 @@ def testViewImageOrFile(self):
373373
response = request.RESPONSE
374374
result = self.file.index_html(request, response)
375375
self.assertEqual(result, self.data)
376+
self.assertIsNone(response.getHeader("Content-Disposition"))
376377

377378
def test_interfaces(self):
378379
from OFS.Image import Image
@@ -387,6 +388,61 @@ def test_text_representation_is_tag(self):
387388
' alt="" title="" height="16" width="16" />')
388389

389390

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

0 commit comments

Comments
 (0)