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

CHANGES.rst

@@ -10,7 +10,12 @@ https://zope.readthedocs.io/en/2.13/CHANGES.html
 4.8.10 (unreleased)
 -------------------
 
-- Nothing changed yet.
+- 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 <https://github.com/zopefoundation/Zope/security/advisories/GHSA-wm8q-9975-xh5v>`_.
 
 
 4.8.9 (2023-09-05)

src/OFS/Image.py

@@ -13,6 +13,7 @@
 """Image object
 """
 
+import os
 import struct
 from email.generator import _make_boundary
 from io import BytesIO
@@ -23,6 +24,7 @@
 from six import PY2
 from six import binary_type
 from six import text_type
+from six.moves.urllib.parse import quote
 
 import ZPublisher.HTTPRequest
 from AccessControl.class_init import InitializeClass
@@ -61,6 +63,42 @@
     from cgi import escape
 
 
+ALLOWED_INLINE_MIMETYPES = [
+    "image/gif",
+    # The mimetypes registry lists several for jpeg 2000:
+    "image/jp2",
+    "image/jpeg",
+    "image/jpeg2000-image",
+    "image/jpeg2000",
+    "image/jpx",
+    "image/png",
+    "image/webp",
+    "image/x-icon",
+    "image/x-jpeg2000-image",
+    "text/plain",
+    # By popular request we allow PDF:
+    "application/pdf",
+]
+
+# Perhaps a denylist is better.
+DISALLOWED_INLINE_MIMETYPES = [
+    "application/javascript",
+    "application/x-javascript",
+    "text/javascript",
+    "text/html",
+    "image/svg+xml",
+    "image/svg+xml-compressed",
+]
+
+# By default we use the allowlist.  We give integrators the option to choose
+# the denylist via an environment variable.
+try:
+    USE_DENYLIST = os.environ.get("OFS_IMAGE_USE_DENYLIST")
+    USE_DENYLIST = bool(int(USE_DENYLIST))
+except (ValueError, TypeError, AttributeError):
+    USE_DENYLIST = False
+
+
 manage_addFileForm = DTMLFile(
     'dtml/imageAdd',
     globals(),
@@ -120,6 +158,13 @@ class File(
     Cacheable
 ):
     """A File object is a content object for arbitrary files."""
+    # You can control which mimetypes may be shown inline
+    # and which must always be downloaded, for security reasons.
+    # Make the configuration available on the class.
+    # Then subclasses can override this.
+    allowed_inline_mimetypes = ALLOWED_INLINE_MIMETYPES
+    disallowed_inline_mimetypes = DISALLOWED_INLINE_MIMETYPES
+    use_denylist = USE_DENYLIST
 
     meta_type = 'File'
     zmi_icon = 'far fa-file-archive'
@@ -418,6 +463,19 @@ def _range_request_handler(self, REQUEST, RESPONSE):
                         b'\r\n--' + boundary.encode('ascii') + b'--\r\n')
                     return True
 
+    def _should_force_download(self):
+        # If this returns True, the caller should set a
+        # Content-Disposition header with filename.
+        mimetype = self.content_type
+        if not mimetype:
+            return False
+        if self.use_denylist:
+            # We explicitly deny a few mimetypes, and allow the rest.
+            return mimetype in self.disallowed_inline_mimetypes
+        # Use the allowlist.
+        # We only explicitly allow a few mimetypes, and deny the rest.
+        return mimetype not in self.allowed_inline_mimetypes
+
     @security.protected(View)
     def index_html(self, REQUEST, RESPONSE):
         """
@@ -456,6 +514,19 @@ def index_html(self, REQUEST, RESPONSE):
         RESPONSE.setHeader('Content-Length', self.size)
         RESPONSE.setHeader('Accept-Ranges', 'bytes')
 
+        if self._should_force_download():
+            # We need a filename, even a dummy one if needed.
+            filename = self.getId()
+            if "." not in filename:
+                # image/svg+xml -> svg
+                ext = self.content_type.split("/")[-1].split("+")[0]
+                filename += "." + ext
+            filename = quote(filename.encode("utf8"))
+            RESPONSE.setHeader(
+                "Content-Disposition",
+                "attachment; filename*=UTF-8''{}".format(filename),
+            )
+
         if self.ZCacheable_isCachingEnabled():
             result = self.ZCacheable_get(default=None)
             if result is not None:

src/OFS/tests/testFileAndImage.py

@@ -368,6 +368,7 @@ def testViewImageOrFile(self):
         response = request.RESPONSE
         result = self.file.index_html(request, response)
         self.assertEqual(result, self.data)
+        self.assertIsNone(response.getHeader("Content-Disposition"))
 
     def test_interfaces(self):
         from OFS.Image import Image
@@ -382,6 +383,20 @@ def test_text_representation_is_tag(self):
                          ' alt="" title="" height="16" width="16" />')
 
 
+class SVGTests(ImageTests):
+    content_type = 'image/svg+xml'
+
+    def testViewImageOrFile(self):
+        request = self.app.REQUEST
+        response = request.RESPONSE
+        result = self.file.index_html(request, response)
+        self.assertEqual(result, self.data)
+        self.assertEqual(
+            response.getHeader("Content-Disposition"),
+            "attachment; filename*=UTF-8''file.svg",
+        )
+
+
 class FileEditTests(Testing.ZopeTestCase.FunctionalTestCase):
     """Browser testing ..Image.File"""