Fix stored XSS in website and author fields

Single quotes were not HTML-escaped in the website field due to
escape() being called with quote=False. This allowed an attacker to
break out of a single-quoted HTML attribute (e.g. href='...') and
inject event handlers such as onmouseover.

The same escaping was missing entirely from the user edit endpoint
and the moderation edit endpoint.

Fix by using escape(..., quote=True) for the website field and
escape(..., quote=False) for author across all three write paths:
POST /new, PUT /id/<id>, and POST /id/<id>/edit/<key>.

Reported-by: ByamB4 <byamba4life@gmail.com>

Author: jelmer

isso/tests/test_comments.py

@@ -69,6 +69,22 @@ def testCreate(self):
         self.assertEqual(rv["mode"], 1)
         self.assertEqual(rv["text"], '<p>Lorem ipsum ...</p>')
 
+    def testWebsiteXSSPayloadIsEscaped(self):
+        """Website field with XSS payload must have quotes HTML-escaped."""
+        payload = "http://x.com/?'onmouseover='alert(document.domain)'x='"
+        rv = self.post('/new?uri=%2Fpath%2F',
+                       data=json.dumps({'text': 'Hello', 'website': payload}))
+        self.assertEqual(rv.status_code, 201)
+        rv = loads(rv.data)
+        # Single quotes must be HTML-escaped so they cannot break out of an
+        # HTML attribute context (e.g. href='...')
+        self.assertNotIn("'", rv["website"])
+        self.assertNotIn('"', rv["website"])
+        self.assertEqual(
+            rv["website"],
+            "http://x.com/?&#x27;onmouseover=&#x27;alert(document.domain)&#x27;x=&#x27;",
+        )
+
     def textCreateWithNonAsciiText(self):
 
         rv = self.post('/new?uri=%2Fpath%2F',
@@ -391,6 +407,27 @@ def testUpdate(self):
         self.assertEqual(rv['website'], 'http://example.com/')
         self.assertIn('modified', rv)
 
+    def testUpdateWebsiteXSSPayloadIsEscaped(self):
+        """Website and author XSS payloads via edit endpoint must be HTML-escaped."""
+        self.post('/new?uri=%2Fpath%2F', data=json.dumps({'text': 'Lorem ipsum ...'}))
+
+        website_payload = "http://x.com/?'onmouseover='alert(document.domain)'x='"
+        author_payload = "<script>alert(1)</script>"
+        self.put('/id/1', data=json.dumps({
+            'text': 'Hello World',
+            'author': author_payload,
+            'website': website_payload,
+        }))
+
+        rv = loads(self.get('/id/1?plain=1').data)
+        self.assertNotIn("'", rv["website"])
+        self.assertEqual(
+            rv["website"],
+            "http://x.com/?&#x27;onmouseover=&#x27;alert(document.domain)&#x27;x=&#x27;",
+        )
+        self.assertNotIn("<script>", rv["author"])
+        self.assertEqual(rv["author"], "&lt;script&gt;alert(1)&lt;/script&gt;")
+
     def testUpdateForbidden(self):
 
         self.post('/new?uri=test', data=json.dumps({'text': 'Hello world!'}))
@@ -860,6 +897,34 @@ def testModerateComment(self):
         # Comment should no longer exist
         self.assertEqual(self.app.db.comments.get(id_), None)
 
+    def testModerateEditXSSPayloadIsEscaped(self):
+        """XSS payloads in author/website via moderate edit endpoint must be HTML-escaped."""
+        id_ = 1
+        signed = self.app.sign(id_)
+
+        self.client.post('/new?uri=/moderated', data=json.dumps({"text": "..."}))
+
+        website_payload = "http://x.com/?'onmouseover='alert(document.domain)'x='"
+        author_payload = "<script>alert(1)</script>"
+        rv = self.client.post(
+            '/id/%d/edit/%s' % (id_, signed),
+            data=json.dumps({
+                "text": "new text",
+                "author": author_payload,
+                "website": website_payload,
+            }),
+        )
+        self.assertEqual(rv.status_code, 200)
+
+        stored = self.app.db.comments.get(id_)
+        self.assertNotIn("'", stored["website"])
+        self.assertEqual(
+            stored["website"],
+            "http://x.com/?&#x27;onmouseover=&#x27;alert(document.domain)&#x27;x=&#x27;",
+        )
+        self.assertNotIn("<script>", stored["author"])
+        self.assertEqual(stored["author"], "&lt;script&gt;alert(1)&lt;/script&gt;")
+
 
 class TestUnsubscribe(unittest.TestCase):
 

isso/views/comments.py

@@ -335,10 +335,13 @@ def new(self, environ, request, uri):
         if not valid:
             return BadRequest(reason)
 
-        for field in ("author", "email", "website"):
+        for field in ("author", "email"):
             if data.get(field) is not None:
                 data[field] = escape(data[field], quote=False)
 
+        if data.get("website") is not None:
+            data["website"] = escape(data["website"], quote=True)
+
         if data.get("website"):
             data["website"] = normalize(data["website"])
 
@@ -548,6 +551,13 @@ def edit(self, environ, request, id):
         if not valid:
             return BadRequest(reason)
 
+        for field in ("author",):
+            if data.get(field) is not None:
+                data[field] = escape(data[field], quote=False)
+
+        if data.get("website") is not None:
+            data["website"] = escape(data["website"], quote=True)
+
         data['modified'] = time.time()
 
         with self.isso.lock:
@@ -794,6 +804,21 @@ def moderate(self, environ, request, id, action, key):
             return Response("Comment has been activated", 200)
         elif action == "edit":
             data = request.json
+
+            for key in set(data.keys()) - set(["text", "author", "website"]):
+                data.pop(key)
+
+            valid, reason = API.verify(data)
+            if not valid:
+                return BadRequest(reason)
+
+            for field in ("author",):
+                if data.get(field) is not None:
+                    data[field] = escape(data[field], quote=False)
+
+            if data.get("website") is not None:
+                data["website"] = escape(data["website"], quote=True)
+
             with self.isso.lock:
                 rv = self.comments.update(id, data)
             for key in set(rv.keys()) - API.FIELDS: