[fc] Repository: plonetheme.barceloneta

Branch: refs/heads/master
Date: 2023-09-21T15:08:07+02:00
Author: Peter Mathis (petschki) <[email protected]>
Commit: plone/plonetheme.barceloneta@2647f3a

typo in CHANGES

Files changed:
M CHANGES.md

last_commit.txt

@@ -1,27 +1,15 @@
-Repository: plone.restapi
+Repository: plonetheme.barceloneta
 
 
-Branch: refs/heads/8.x.x
-Date: 2023-09-21T13:33:35+02:00
-Author: Maurits van Rees (mauritsvanrees) <[email protected]>
-Commit: https://github.com/plone/plone.restapi/commit/f280675e445a86cbede2a7bf2b5ff65ce30a9a31
+Branch: refs/heads/master
+Date: 2023-09-21T15:08:07+02:00
+Author: Peter Mathis (petschki) <[email protected]>
+Commit: https://github.com/plone/plonetheme.barceloneta/commit/2647f3ae089a65616d7c744d95b6c1a8da9797fa
 
-Merge pull request from GHSA-hc5c-r8m5-2gfh
-
-* Fix stored XSS (Cross Site Scripting) for SVG image in user portrait.
-
-Done by forcing a download instead of displaying inline.
-Normal accessing via an image tag is not affected and is safe.
-
-* Fix portrait tests.
-
-The content type is not recognized correctly during upload.
+typo in CHANGES
 
 Files changed:
-A news/1.bugfix
-A src/plone/restapi/tests/image.svg
-M src/plone/restapi/services/users/get.py
-M src/plone/restapi/tests/test_services_users.py
+M CHANGES.md
 
-b'diff --git a/news/1.bugfix b/news/1.bugfix\nnew file mode 100644\nindex 0000000000..37a92b93ea\n--- /dev/null\n+++ b/news/1.bugfix\n@@ -0,0 +1,5 @@\n+Fix stored XSS (Cross Site Scripting) for SVG image in user portrait.\n+Done by forcing a download instead of displaying inline.\n+Normal accessing via an image tag is not affected and is safe.\n+See `security advisory <https://github.com/plone/plone.restapi/security/advisories/GHSA-hc5c-r8m5-2gfh>`_.\n+[maurits]\ndiff --git a/src/plone/restapi/services/users/get.py b/src/plone/restapi/services/users/get.py\nindex 10ed48af4a..bef0c3ef0e 100644\n--- a/src/plone/restapi/services/users/get.py\n+++ b/src/plone/restapi/services/users/get.py\n@@ -2,6 +2,9 @@\n from Acquisition import aq_inner\n from itertools import chain\n from plone.app.workflow.browser.sharing import merge_search_results\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.utils import stream_data\n from plone.restapi.interfaces import ISerializeToJson\n from plone.restapi.services import Service\n@@ -13,6 +16,7 @@\n from typing import Iterable\n from typing import Sequence\n from urllib.parse import parse_qs\n+from urllib.parse import quote\n from zExceptions import BadRequest\n from zope.component import getMultiAdapter\n from zope.component import queryMultiAdapter\n@@ -220,6 +224,14 @@ def reply(self):\n \n @implementer(IPublishTraverse)\n class PortraitGet(Service):\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):\n         super().__init__(context, request)\n         self.params = []\n@@ -237,6 +249,18 @@ def _get_user_id(self):\n             raise Exception("Must supply exactly one parameter (user id)")\n         return self.params[0]\n \n+    def _should_force_download(self, portrait):\n+        # If this returns True, the caller should set the Content-Disposition header.\n+        mimetype = portrait.content_type\n+        if not mimetype:\n+            return False\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 render(self):\n         if len(self.params) == 1:\n             # Retrieve the user portrait\n@@ -254,6 +278,15 @@ def render(self):\n             self.request.response.setStatus(404)\n             return None\n \n+        if self._should_force_download(portrait):\n+            # We need a filename, even a dummy one if needed.\n+            ext = portrait.content_type.split("/")[-1].split("+")[0]\n+            filename = f"{portrait.getId()}.{ext}"\n+            filename = quote(filename.encode("utf8"))\n+            self.request.response.setHeader(\n+                "Content-Disposition", f"attachment; filename*=UTF-8\'\'{filename}"\n+            )\n+\n         self.request.response.setStatus(200)\n         self.request.response.setHeader("Content-Type", portrait.content_type)\n \ndiff --git a/src/plone/restapi/tests/image.svg b/src/plone/restapi/tests/image.svg\nnew file mode 100644\nindex 0000000000..18d6411909\n--- /dev/null\n+++ b/src/plone/restapi/tests/image.svg\n@@ -0,0 +1,3 @@\n+<svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" fill="currentColor" class="bi bi-person" viewBox="0 0 16 16">\n+  <path d="M8 8a3 3 0 1 0 0-6 3 3 0 0 0 0 6Zm2-3a2 2 0 1 1-4 0 2 2 0 0 1 4 0Zm4 8c0 1-1 1-1 1H3s-1 0-1-1 1-4 6-4 6 3 6 4Zm-1-.004c-.001-.246-.154-.986-.832-1.664C11.516 10.68 10.289 10 8 10c-2.29 0-3.516.68-4.168 1.332-.678.678-.83 1.418-.832 1.664h10Z"/>\n+</svg>\n\\ No newline at end of file\ndiff --git a/src/plone/restapi/tests/test_services_users.py b/src/plone/restapi/tests/test_services_users.py\nindex b08170eafc..9555745a67 100644\n--- a/src/plone/restapi/tests/test_services_users.py\n+++ b/src/plone/restapi/tests/test_services_users.py\n@@ -16,6 +16,7 @@\n from zope.component import getAdapter\n from zope.component import getUtility\n \n+import base64\n import os\n import re\n import transaction\n@@ -530,6 +531,37 @@ def test_update_portrait(self):\n         self.assertEqual(response.status_code, 204)\n         transaction.commit()\n \n+    def _update_portrait_with_svg(self):\n+        here = os.path.dirname(__file__)\n+        # icon from https://icons.getbootstrap.com/icons/person/\n+        path = os.path.join(here, "image.svg")\n+        with open(path, "rb") as image:\n+            data = base64.encodebytes(image.read())\n+\n+        payload = {\n+            "portrait": {\n+                "filename": "image.svg",\n+                "encoding": "base64",\n+                "data": data,\n+                "content-type": "image/svg+xml",\n+            }\n+        }\n+        self.api_session.auth = ("noam", "password")\n+        response = self.api_session.patch("/@users/noam", json=payload)\n+\n+        self.assertEqual(response.status_code, 204)\n+        transaction.commit()\n+        pmd = api.portal.get_tool("portal_memberdata")\n+        # The mimetype is not correctly recognized, at least in the tests.\n+        portrait = pmd.portraits.noam\n+        if portrait.content_type == "text/html":\n+            portrait.content_type = "image/svg+xml"\n+            transaction.commit()\n+\n+\n+    def test_update_portrait_with_svg(self):\n+        self._update_portrait_with_svg()\n+\n         user = self.api_session.get("/@users/noam").json()\n         self.assertTrue(user.get("portrait").endswith("/@portrait/noam"))\n \n@@ -1072,6 +1104,23 @@ def test_get_own_user_portrait(self):\n         self.assertEqual(response.headers["Content-Type"], "image/gif")\n         noam_api_session.close()\n \n+    def test_get_own_user_portrait_with_svg(self):\n+        self._update_portrait_with_svg()\n+\n+        noam_api_session = RelativeSession(self.portal_url, test=self)\n+        noam_api_session.headers.update({"Accept": "application/json"})\n+        noam_api_session.auth = ("noam", "password")\n+\n+        response = noam_api_session.get("/@portrait")\n+\n+        self.assertEqual(200, response.status_code)\n+        self.assertEqual(response.headers["Content-Type"], "image/svg+xml")\n+        self.assertEqual(\n+            response.headers["Content-Disposition"],\n+            "attachment; filename*=UTF-8\'\'noam.svg",\n+        )\n+        noam_api_session.close()\n+\n     def test_get_own_user_portrait_logged_out(self):\n         response = self.anon_api_session.get(\n             "/@portrait",\n@@ -1089,6 +1138,9 @@ def test_get_user_portrait_not_set(self):\n     def test_get_user_portrait(self):\n         with self.makeRealImage() as image:\n             pm = api.portal.get_tool("portal_membership")\n+            # Note: if you would set an SVG in this way, this would give a\n+            # PIL.UnidentifiedImageError, which is what happens in ClassicUI\n+            # as well.\n             pm.changeMemberPortrait(image, "noam")\n             transaction.commit()\n \n@@ -1098,6 +1150,26 @@ def test_get_user_portrait(self):\n \n         self.assertEqual(200, response.status_code)\n         self.assertEqual(response.headers["Content-Type"], "image/gif")\n+        self.assertIsNone(response.headers.get("Content-Disposition"))\n+\n+    def test_get_user_portrait_with_svg(self):\n+        # If we would upload an SVG in the same way as in\n+        # test_get_user_portrait, with pm.changeMemberPortrait,\n+        # this would actually give PIL.UnidentifiedImageError,\n+        # which is what happens in ClassicUI as well.\n+        # So update it with a restapi call instead.\n+        self._update_portrait_with_svg()\n+\n+        response = self.api_session.get(\n+            "/@portrait/noam",\n+        )\n+\n+        self.assertEqual(200, response.status_code)\n+        self.assertEqual(response.headers["Content-Type"], "image/svg+xml")\n+        self.assertEqual(\n+            response.headers["Content-Disposition"],\n+            "attachment; filename*=UTF-8\'\'noam.svg",\n+        )\n \n     def test_get_user_portrait_anonymous(self):\n         with self.makeRealImage() as image:\n@@ -1111,6 +1183,7 @@ def test_get_user_portrait_anonymous(self):\n \n         self.assertEqual(200, response.status_code)\n         self.assertEqual(response.headers["Content-Type"], "image/gif")\n+        self.assertIsNone(response.headers.get("Content-Disposition"))\n \n     def test_get_user_portrait_if_email_login_enabled(self):\n         # enable use_email_as_login\n'
+b'diff --git a/CHANGES.md b/CHANGES.md\nindex 3112e538..3620e42d 100644\n--- a/CHANGES.md\n+++ b/CHANGES.md\n@@ -14,7 +14,7 @@\n \n ### Bug fixes:\n \n-- Update Bootstrap to ``5.2.3``\n+- Update Bootstrap to ``5.3.2``\n   [petschki] #346\n \n \n'