From a92e703ab9d78aecc062e797f941bb7e206650a5 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <clokep@users.noreply.github.com>
Date: Mon, 10 Feb 2020 16:35:26 -0500
Subject: [PATCH] Reject device display names that are too long (#6882)

* Reject device display names that are too long.

Too long is currently defined as 100 characters in length.

* Add a regression test for rejecting a too long device display name.
---
 changelog.d/6882.misc         |  1 +
 synapse/handlers/device.py    | 14 +++++++++++++-
 tests/handlers/test_device.py | 18 ++++++++++++++++++
 3 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 changelog.d/6882.misc

diff --git a/changelog.d/6882.misc b/changelog.d/6882.misc
new file mode 100644
index 000000000..e8382e36a
--- /dev/null
+++ b/changelog.d/6882.misc
@@ -0,0 +1 @@
+Reject device display names over 100 characters in length.
diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py
index 6d8e48ed3..50cea3f37 100644
--- a/synapse/handlers/device.py
+++ b/synapse/handlers/device.py
@@ -26,6 +26,7 @@ from synapse.api.errors import (
     FederationDeniedError,
     HttpResponseException,
     RequestSendFailed,
+    SynapseError,
 )
 from synapse.logging.opentracing import log_kv, set_tag, trace
 from synapse.types import RoomStreamToken, get_domain_from_id
@@ -39,6 +40,8 @@ from ._base import BaseHandler
 
 logger = logging.getLogger(__name__)
 
+MAX_DEVICE_DISPLAY_NAME_LEN = 100
+
 
 class DeviceWorkerHandler(BaseHandler):
     def __init__(self, hs):
@@ -404,9 +407,18 @@ class DeviceHandler(DeviceWorkerHandler):
             defer.Deferred:
         """
 
+        # Reject a new displayname which is too long.
+        new_display_name = content.get("display_name")
+        if new_display_name and len(new_display_name) > MAX_DEVICE_DISPLAY_NAME_LEN:
+            raise SynapseError(
+                400,
+                "Device display name is too long (max %i)"
+                % (MAX_DEVICE_DISPLAY_NAME_LEN,),
+            )
+
         try:
             yield self.store.update_device(
-                user_id, device_id, new_display_name=content.get("display_name")
+                user_id, device_id, new_display_name=new_display_name
             )
             yield self.notify_device_update(user_id, [device_id])
         except errors.StoreError as e:
diff --git a/tests/handlers/test_device.py b/tests/handlers/test_device.py
index a3aa0a1cf..62b47f657 100644
--- a/tests/handlers/test_device.py
+++ b/tests/handlers/test_device.py
@@ -160,6 +160,24 @@ class DeviceTestCase(unittest.HomeserverTestCase):
         res = self.get_success(self.handler.get_device(user1, "abc"))
         self.assertEqual(res["display_name"], "new display")
 
+    def test_update_device_too_long_display_name(self):
+        """Update a device with a display name that is invalid (too long)."""
+        self._record_users()
+
+        # Request to update a device display name with a new value that is longer than allowed.
+        update = {
+            "display_name": "a"
+            * (synapse.handlers.device.MAX_DEVICE_DISPLAY_NAME_LEN + 1)
+        }
+        self.get_failure(
+            self.handler.update_device(user1, "abc", update),
+            synapse.api.errors.SynapseError,
+        )
+
+        # Ensure the display name was not updated.
+        res = self.get_success(self.handler.get_device(user1, "abc"))
+        self.assertEqual(res["display_name"], "display 2")
+
     def test_update_unknown_device(self):
         update = {"display_name": "new_display"}
         res = self.handler.update_device("user_id", "unknown_device_id", update)