diff --git a/last_commit.txt b/last_commit.txt index 664cafc931..f4633762d6 100644 --- a/last_commit.txt +++ b/last_commit.txt @@ -1,40 +1,16 @@ -Repository: plone.namedfile +Repository: plone.app.upgrade -Branch: refs/heads/5.x -Date: 2023-09-18T14:05:37+02:00 +Branch: refs/heads/2.x +Date: 2023-09-21T20:21:11+02:00 Author: Maurits van Rees (mauritsvanrees) -Commit: https://github.com/plone/plone.namedfile/commit/776cb9b54241adff1fccd940c3d6abed35ca83e4 +Commit: https://github.com/plone/plone.app.upgrade/commit/721cad77d0af159ea5fac7dfb43e7f6ee830ca7c -Force download of image scales when inline is not allowed for this mimetype. - -We already did this for the `@@display-file` view in 2021, but now it is done for image scales as well. - -See [security advisory](https://github.com/plone/plone.namedfile/security/advisories/GHSA-jj7c-jrv4-c65x). - -Files changed: -A news/1.bugfix -M plone/namedfile/scaling.py -M plone/namedfile/tests/test_display_file.py - -b'diff --git a/news/1.bugfix b/news/1.bugfix\nnew file mode 100644\nindex 0000000..c6cf8bb\n--- /dev/null\n+++ b/news/1.bugfix\n@@ -0,0 +1,4 @@\n+Fix stored XSS (Cross Site Scripting) for SVG images.\n+Done by forcing a download instead of displaying inline.\n+See `security advisory `_.\n+[maurits]\ndiff --git a/plone/namedfile/scaling.py b/plone/namedfile/scaling.py\nindex 30d7848..27089ab 100644\n--- a/plone/namedfile/scaling.py\n+++ b/plone/namedfile/scaling.py\n@@ -5,6 +5,9 @@\n from io import BytesIO\n from plone.memoize import ram\n from plone.namedfile.file import FILECHUNK_CLASSES\n+from plone.namedfile.browser import ALLOWED_INLINE_MIMETYPES\n+from plone.namedfile.browser import DISALLOWED_INLINE_MIMETYPES\n+from plone.namedfile.browser import USE_DENYLIST\n from plone.namedfile.interfaces import IAvailableSizes\n from plone.namedfile.interfaces import IStableImageScale\n from plone.namedfile.utils import getHighPixelDensityScales\n@@ -51,6 +54,14 @@ class ImageScale(BrowserView):\n __allow_access_to_unprotected_subobjects__ = 1\n data = None\n \n+ # You can control which mimetypes may be shown inline\n+ # and which must always be downloaded, for security reasons.\n+ # Make the configuration available on the class.\n+ # Then subclasses can override this.\n+ allowed_inline_mimetypes = ALLOWED_INLINE_MIMETYPES\n+ disallowed_inline_mimetypes = DISALLOWED_INLINE_MIMETYPES\n+ use_denylist = USE_DENYLIST\n+\n def __init__(self, context, request, **info):\n self.context = context\n self.request = request\n@@ -138,10 +149,37 @@ def validate_access(self):\n fieldname = getattr(self.data, "fieldname", getattr(self, "fieldname", None))\n guarded_getattr(self.context, fieldname)\n \n+ def _should_force_download(self):\n+ # If this returns True, the caller should call set_headers with a filename.\n+ if not hasattr(self.data, "contentType"):\n+ return\n+ mimetype = self.data.contentType\n+ if self.use_denylist:\n+ # We explicitly deny a few mimetypes, and allow the rest.\n+ return mimetype in self.disallowed_inline_mimetypes\n+ # Use the allowlist.\n+ # We only explicitly allow a few mimetypes, and deny the rest.\n+ return mimetype not in self.allowed_inline_mimetypes\n+\n+ def set_headers(self, response=None):\n+ # set headers for the image\n+ image = self.data\n+ if response is None:\n+ response = self.request.response\n+ filename = None\n+ if self._should_force_download():\n+ # We MUST pass a filename, even a dummy one if needed.\n+ filename = getattr(image, "filename", getattr(self, "filename", None))\n+ if filename is None:\n+ filename = getattr(image, "fieldname", getattr(self, "fieldname", None))\n+ if filename is None:\n+ filename = "image.ext"\n+ set_headers(image, response, filename=filename)\n+\n def index_html(self):\n """ download the image """\n self.validate_access()\n- set_headers(self.data, self.request.response)\n+ self.set_headers()\n return stream_data(self.data)\n \n def manage_DAVget(self):\n@@ -161,7 +199,7 @@ def HEAD(self, REQUEST, RESPONSE=None):\n without transfer of the image itself\n """\n self.validate_access()\n- set_headers(self.data, REQUEST.response)\n+ self.set_headers(response=REQUEST.response)\n return ""\n \n HEAD.__roles__ = ("Anonymous",)\ndiff --git a/plone/namedfile/tests/test_display_file.py b/plone/namedfile/tests/test_display_file.py\nindex fe537e8..9a2422a 100644\n--- a/plone/namedfile/tests/test_display_file.py\n+++ b/plone/namedfile/tests/test_display_file.py\n@@ -4,12 +4,15 @@\n from plone.app.testing import SITE_OWNER_PASSWORD\n from plone.namedfile import field\n from plone.namedfile import file\n+from plone.namedfile.interfaces import IAvailableSizes\n from plone.namedfile.interfaces import IImageScaleTraversable\n from plone.namedfile.testing import PLONE_NAMEDFILE_FUNCTIONAL_TESTING\n from plone.namedfile.tests import getFile\n from plone.testing.zope import Browser\n from Products.CMFPlone.utils import safe_unicode\n+from zope.annotation import IAnnotations\n from zope.annotation import IAttributeAnnotatable\n+from zope.component import getSiteManager\n from zope.interface import implementer\n \n import os\n@@ -48,6 +51,11 @@ def get_disposition_header(browser):\n return browser.headers.get(name, None)\n \n \n+def custom_available_sizes():\n+ # Define available image scales.\n+ return {"custom": (10, 10)}\n+\n+\n class TestAttackVectorNamedImage(unittest.TestCase):\n layer = PLONE_NAMEDFILE_FUNCTIONAL_TESTING\n field_class = file.NamedImage\n@@ -58,6 +66,12 @@ def setUp(self):\n item = DummyContent()\n self.layer["app"]._setOb("item", item)\n self.item = self.layer["app"].item\n+ sm = getSiteManager()\n+ sm.registerUtility(component=custom_available_sizes, provided=IAvailableSizes)\n+\n+ def tearDown(self):\n+ sm = getSiteManager()\n+ sm.unregisterUtility(provided=IAvailableSizes)\n \n def get_admin_browser(self):\n browser = Browser(self.layer["app"])\n@@ -100,12 +114,50 @@ def assert_display_inline_is_download(self, base_url):\n self.assertIn("attachment", header)\n self.assertIn("filename", header)\n \n+ def assert_scale_view_works(self, base_url):\n+ # Test that accessing a scale view shows the image inline.\n+ browser = self.get_anon_browser()\n+ browser.open(base_url + "/@@images/{}".format(self.field_name))\n+ self.assertIsNone(get_disposition_header(browser))\n+\n+ # Note: the \'custom\' scale is defined in an adapter above.\n+ browser.open(base_url + "/@@images/{}/custom".format(self.field_name))\n+ self.assertIsNone(get_disposition_header(browser))\n+\n+ unique_scale_id = list(IAnnotations(self.item)["plone.scale"].keys())[0]\n+ browser.open(base_url + "/@@images/{}".format(unique_scale_id))\n+ self.assertIsNone(get_disposition_header(browser))\n+\n+ def assert_scale_view_is_download(self, base_url):\n+ # Test that accessing a scale view turns into a download.\n+ browser = self.get_anon_browser()\n+ browser.open(base_url + "/@@images/{}".format(self.field_name))\n+ header = get_disposition_header(browser)\n+ self.assertIsNotNone(header)\n+ self.assertIn("attachment", header)\n+ self.assertIn("filename", header)\n+\n+ browser.open(base_url + "/@@images/{}/custom".format(self.field_name))\n+ header = get_disposition_header(browser)\n+ self.assertIsNotNone(header)\n+ self.assertIn("attachment", header)\n+ self.assertIn("filename", header)\n+\n+ unique_scale_id = list(IAnnotations(self.item)["plone.scale"].keys())[0]\n+ browser.open(base_url + "/@@images/{}".format(unique_scale_id))\n+ header = get_disposition_header(browser)\n+ self.assertIsNotNone(header)\n+ self.assertIn("attachment", header)\n+ self.assertIn("filename", header)\n+\n def test_png_image(self):\n setattr(self.item, self.field_name, self._named_file("image.png"))\n transaction.commit()\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_works(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_works(base_url)\n \n def test_svg_image(self):\n setattr(self.item, self.field_name, self._named_file("image.svg"))\n@@ -113,10 +165,13 @@ def test_svg_image(self):\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_is_download(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_is_download(base_url)\n \n def test_filename_none(self):\n- # A \'None\' filename None probably does not happen during normal upload,\n- # but if an attacker manages this, even @@download will show inline.\n+ # A \'None\' filename probably does not happen during normal upload,\n+ # but if an attacker manages this, even @@download would show inline.\n+ # We prevent this.\n data = self._named_file("image.svg")\n data.filename = None\n setattr(self.item, self.field_name, data)\n@@ -124,6 +179,8 @@ def test_filename_none(self):\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_is_download(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_is_download(base_url)\n \n def test_filename_empty(self):\n # An empty filename is probably no problem, but let\'s check.\n@@ -134,6 +191,8 @@ def test_filename_empty(self):\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_is_download(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_is_download(base_url)\n \n \n class TestAttackVectorNamedBlobImage(TestAttackVectorNamedImage):\n' - -Repository: plone.namedfile - - -Branch: refs/heads/5.x -Date: 2023-09-21T13:22:31+02:00 -Author: Maurits van Rees (mauritsvanrees) -Commit: https://github.com/plone/plone.namedfile/commit/ff5269fb4c79f4eb91dd934561b8824a49a03b60 - -Merge pull request from GHSA-jj7c-jrv4-c65x - -Force download of image scales when inline is not allowed for mime type [5.x] +Added upgrade to 5223, Plone 5.2.15. Files changed: -A news/1.bugfix -M plone/namedfile/scaling.py -M plone/namedfile/tests/test_display_file.py +A news/5223.bugfix +M plone/app/upgrade/v52/configure.zcml -b'diff --git a/news/1.bugfix b/news/1.bugfix\nnew file mode 100644\nindex 0000000..c6cf8bb\n--- /dev/null\n+++ b/news/1.bugfix\n@@ -0,0 +1,4 @@\n+Fix stored XSS (Cross Site Scripting) for SVG images.\n+Done by forcing a download instead of displaying inline.\n+See `security advisory `_.\n+[maurits]\ndiff --git a/plone/namedfile/scaling.py b/plone/namedfile/scaling.py\nindex 30d7848..27089ab 100644\n--- a/plone/namedfile/scaling.py\n+++ b/plone/namedfile/scaling.py\n@@ -5,6 +5,9 @@\n from io import BytesIO\n from plone.memoize import ram\n from plone.namedfile.file import FILECHUNK_CLASSES\n+from plone.namedfile.browser import ALLOWED_INLINE_MIMETYPES\n+from plone.namedfile.browser import DISALLOWED_INLINE_MIMETYPES\n+from plone.namedfile.browser import USE_DENYLIST\n from plone.namedfile.interfaces import IAvailableSizes\n from plone.namedfile.interfaces import IStableImageScale\n from plone.namedfile.utils import getHighPixelDensityScales\n@@ -51,6 +54,14 @@ class ImageScale(BrowserView):\n __allow_access_to_unprotected_subobjects__ = 1\n data = None\n \n+ # You can control which mimetypes may be shown inline\n+ # and which must always be downloaded, for security reasons.\n+ # Make the configuration available on the class.\n+ # Then subclasses can override this.\n+ allowed_inline_mimetypes = ALLOWED_INLINE_MIMETYPES\n+ disallowed_inline_mimetypes = DISALLOWED_INLINE_MIMETYPES\n+ use_denylist = USE_DENYLIST\n+\n def __init__(self, context, request, **info):\n self.context = context\n self.request = request\n@@ -138,10 +149,37 @@ def validate_access(self):\n fieldname = getattr(self.data, "fieldname", getattr(self, "fieldname", None))\n guarded_getattr(self.context, fieldname)\n \n+ def _should_force_download(self):\n+ # If this returns True, the caller should call set_headers with a filename.\n+ if not hasattr(self.data, "contentType"):\n+ return\n+ mimetype = self.data.contentType\n+ if self.use_denylist:\n+ # We explicitly deny a few mimetypes, and allow the rest.\n+ return mimetype in self.disallowed_inline_mimetypes\n+ # Use the allowlist.\n+ # We only explicitly allow a few mimetypes, and deny the rest.\n+ return mimetype not in self.allowed_inline_mimetypes\n+\n+ def set_headers(self, response=None):\n+ # set headers for the image\n+ image = self.data\n+ if response is None:\n+ response = self.request.response\n+ filename = None\n+ if self._should_force_download():\n+ # We MUST pass a filename, even a dummy one if needed.\n+ filename = getattr(image, "filename", getattr(self, "filename", None))\n+ if filename is None:\n+ filename = getattr(image, "fieldname", getattr(self, "fieldname", None))\n+ if filename is None:\n+ filename = "image.ext"\n+ set_headers(image, response, filename=filename)\n+\n def index_html(self):\n """ download the image """\n self.validate_access()\n- set_headers(self.data, self.request.response)\n+ self.set_headers()\n return stream_data(self.data)\n \n def manage_DAVget(self):\n@@ -161,7 +199,7 @@ def HEAD(self, REQUEST, RESPONSE=None):\n without transfer of the image itself\n """\n self.validate_access()\n- set_headers(self.data, REQUEST.response)\n+ self.set_headers(response=REQUEST.response)\n return ""\n \n HEAD.__roles__ = ("Anonymous",)\ndiff --git a/plone/namedfile/tests/test_display_file.py b/plone/namedfile/tests/test_display_file.py\nindex fe537e8..9a2422a 100644\n--- a/plone/namedfile/tests/test_display_file.py\n+++ b/plone/namedfile/tests/test_display_file.py\n@@ -4,12 +4,15 @@\n from plone.app.testing import SITE_OWNER_PASSWORD\n from plone.namedfile import field\n from plone.namedfile import file\n+from plone.namedfile.interfaces import IAvailableSizes\n from plone.namedfile.interfaces import IImageScaleTraversable\n from plone.namedfile.testing import PLONE_NAMEDFILE_FUNCTIONAL_TESTING\n from plone.namedfile.tests import getFile\n from plone.testing.zope import Browser\n from Products.CMFPlone.utils import safe_unicode\n+from zope.annotation import IAnnotations\n from zope.annotation import IAttributeAnnotatable\n+from zope.component import getSiteManager\n from zope.interface import implementer\n \n import os\n@@ -48,6 +51,11 @@ def get_disposition_header(browser):\n return browser.headers.get(name, None)\n \n \n+def custom_available_sizes():\n+ # Define available image scales.\n+ return {"custom": (10, 10)}\n+\n+\n class TestAttackVectorNamedImage(unittest.TestCase):\n layer = PLONE_NAMEDFILE_FUNCTIONAL_TESTING\n field_class = file.NamedImage\n@@ -58,6 +66,12 @@ def setUp(self):\n item = DummyContent()\n self.layer["app"]._setOb("item", item)\n self.item = self.layer["app"].item\n+ sm = getSiteManager()\n+ sm.registerUtility(component=custom_available_sizes, provided=IAvailableSizes)\n+\n+ def tearDown(self):\n+ sm = getSiteManager()\n+ sm.unregisterUtility(provided=IAvailableSizes)\n \n def get_admin_browser(self):\n browser = Browser(self.layer["app"])\n@@ -100,12 +114,50 @@ def assert_display_inline_is_download(self, base_url):\n self.assertIn("attachment", header)\n self.assertIn("filename", header)\n \n+ def assert_scale_view_works(self, base_url):\n+ # Test that accessing a scale view shows the image inline.\n+ browser = self.get_anon_browser()\n+ browser.open(base_url + "/@@images/{}".format(self.field_name))\n+ self.assertIsNone(get_disposition_header(browser))\n+\n+ # Note: the \'custom\' scale is defined in an adapter above.\n+ browser.open(base_url + "/@@images/{}/custom".format(self.field_name))\n+ self.assertIsNone(get_disposition_header(browser))\n+\n+ unique_scale_id = list(IAnnotations(self.item)["plone.scale"].keys())[0]\n+ browser.open(base_url + "/@@images/{}".format(unique_scale_id))\n+ self.assertIsNone(get_disposition_header(browser))\n+\n+ def assert_scale_view_is_download(self, base_url):\n+ # Test that accessing a scale view turns into a download.\n+ browser = self.get_anon_browser()\n+ browser.open(base_url + "/@@images/{}".format(self.field_name))\n+ header = get_disposition_header(browser)\n+ self.assertIsNotNone(header)\n+ self.assertIn("attachment", header)\n+ self.assertIn("filename", header)\n+\n+ browser.open(base_url + "/@@images/{}/custom".format(self.field_name))\n+ header = get_disposition_header(browser)\n+ self.assertIsNotNone(header)\n+ self.assertIn("attachment", header)\n+ self.assertIn("filename", header)\n+\n+ unique_scale_id = list(IAnnotations(self.item)["plone.scale"].keys())[0]\n+ browser.open(base_url + "/@@images/{}".format(unique_scale_id))\n+ header = get_disposition_header(browser)\n+ self.assertIsNotNone(header)\n+ self.assertIn("attachment", header)\n+ self.assertIn("filename", header)\n+\n def test_png_image(self):\n setattr(self.item, self.field_name, self._named_file("image.png"))\n transaction.commit()\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_works(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_works(base_url)\n \n def test_svg_image(self):\n setattr(self.item, self.field_name, self._named_file("image.svg"))\n@@ -113,10 +165,13 @@ def test_svg_image(self):\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_is_download(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_is_download(base_url)\n \n def test_filename_none(self):\n- # A \'None\' filename None probably does not happen during normal upload,\n- # but if an attacker manages this, even @@download will show inline.\n+ # A \'None\' filename probably does not happen during normal upload,\n+ # but if an attacker manages this, even @@download would show inline.\n+ # We prevent this.\n data = self._named_file("image.svg")\n data.filename = None\n setattr(self.item, self.field_name, data)\n@@ -124,6 +179,8 @@ def test_filename_none(self):\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_is_download(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_is_download(base_url)\n \n def test_filename_empty(self):\n # An empty filename is probably no problem, but let\'s check.\n@@ -134,6 +191,8 @@ def test_filename_empty(self):\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_is_download(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_is_download(base_url)\n \n \n class TestAttackVectorNamedBlobImage(TestAttackVectorNamedImage):\n' +b'diff --git a/news/5223.bugfix b/news/5223.bugfix\nnew file mode 100644\nindex 00000000..e6e3662e\n--- /dev/null\n+++ b/news/5223.bugfix\n@@ -0,0 +1,2 @@\n+Added upgrade to 5223, Plone 5.2.15.\n+[maurits]\ndiff --git a/plone/app/upgrade/v52/configure.zcml b/plone/app/upgrade/v52/configure.zcml\nindex 032a1d18..76b7e702 100644\n--- a/plone/app/upgrade/v52/configure.zcml\n+++ b/plone/app/upgrade/v52/configure.zcml\n@@ -326,4 +326,17 @@\n \n \n \n+ \n+ \n+\n+ \n+\n+ \n+\n \n'