Merge pull request #5309 from matrix-org/rav/limit_displayname_length

Limit displaynames and avatar URLs
This commit is contained in:
Richard van der Hoff 2019-06-01 11:34:50 +01:00 committed by GitHub
commit d828d1dc57
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 76 additions and 2 deletions

1
changelog.d/5309.bugfix Normal file
View File

@ -0,0 +1 @@
Prevent users from setting huge displaynames and avatar URLs.

View File

@ -31,6 +31,9 @@ from ._base import BaseHandler
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
MAX_DISPLAYNAME_LEN = 100
MAX_AVATAR_URL_LEN = 1000
class BaseProfileHandler(BaseHandler): class BaseProfileHandler(BaseHandler):
"""Handles fetching and updating user profile information. """Handles fetching and updating user profile information.
@ -162,6 +165,11 @@ class BaseProfileHandler(BaseHandler):
if not by_admin and target_user != requester.user: if not by_admin and target_user != requester.user:
raise AuthError(400, "Cannot set another user's displayname") raise AuthError(400, "Cannot set another user's displayname")
if len(new_displayname) > MAX_DISPLAYNAME_LEN:
raise SynapseError(
400, "Displayname is too long (max %i)" % (MAX_DISPLAYNAME_LEN, ),
)
if new_displayname == '': if new_displayname == '':
new_displayname = None new_displayname = None
@ -217,6 +225,11 @@ class BaseProfileHandler(BaseHandler):
if not by_admin and target_user != requester.user: if not by_admin and target_user != requester.user:
raise AuthError(400, "Cannot set another user's avatar_url") raise AuthError(400, "Cannot set another user's avatar_url")
if len(new_avatar_url) > MAX_AVATAR_URL_LEN:
raise SynapseError(
400, "Avatar URL is too long (max %i)" % (MAX_AVATAR_URL_LEN, ),
)
yield self.store.set_profile_avatar_url( yield self.store.set_profile_avatar_url(
target_user.localpart, new_avatar_url target_user.localpart, new_avatar_url
) )

View File

@ -531,6 +531,8 @@ class RegistrationHandler(BaseHandler):
A tuple of (user_id, access_token). A tuple of (user_id, access_token).
Raises: Raises:
RegistrationError if there was a problem registering. RegistrationError if there was a problem registering.
NB this is only used in tests. TODO: move it to the test package!
""" """
if localpart is None: if localpart is None:
raise SynapseError(400, "Request must include user id") raise SynapseError(400, "Request must include user id")

View File

@ -14,6 +14,8 @@
# limitations under the License. # limitations under the License.
"""Tests REST events for /profile paths.""" """Tests REST events for /profile paths."""
import json
from mock import Mock from mock import Mock
from twisted.internet import defer from twisted.internet import defer
@ -31,8 +33,11 @@ myid = "@1234ABCD:test"
PATH_PREFIX = "/_matrix/client/api/v1" PATH_PREFIX = "/_matrix/client/api/v1"
class ProfileTestCase(unittest.TestCase): class MockHandlerProfileTestCase(unittest.TestCase):
""" Tests profile management. """ """ Tests rest layer of profile management.
Todo: move these into ProfileTestCase
"""
@defer.inlineCallbacks @defer.inlineCallbacks
def setUp(self): def setUp(self):
@ -159,6 +164,59 @@ class ProfileTestCase(unittest.TestCase):
self.assertEquals(mocked_set.call_args[0][2], "http://my.server/pic.gif") self.assertEquals(mocked_set.call_args[0][2], "http://my.server/pic.gif")
class ProfileTestCase(unittest.HomeserverTestCase):
servlets = [
admin.register_servlets_for_client_rest_resource,
login.register_servlets,
profile.register_servlets,
]
def make_homeserver(self, reactor, clock):
self.hs = self.setup_test_homeserver()
return self.hs
def prepare(self, reactor, clock, hs):
self.owner = self.register_user("owner", "pass")
self.owner_tok = self.login("owner", "pass")
def test_set_displayname(self):
request, channel = self.make_request(
"PUT",
"/profile/%s/displayname" % (self.owner, ),
content=json.dumps({"displayname": "test"}),
access_token=self.owner_tok,
)
self.render(request)
self.assertEqual(channel.code, 200, channel.result)
res = self.get_displayname()
self.assertEqual(res, "test")
def test_set_displayname_too_long(self):
"""Attempts to set a stupid displayname should get a 400"""
request, channel = self.make_request(
"PUT",
"/profile/%s/displayname" % (self.owner, ),
content=json.dumps({"displayname": "test" * 100}),
access_token=self.owner_tok,
)
self.render(request)
self.assertEqual(channel.code, 400, channel.result)
res = self.get_displayname()
self.assertEqual(res, "owner")
def get_displayname(self):
request, channel = self.make_request(
"GET",
"/profile/%s/displayname" % (self.owner, ),
)
self.render(request)
self.assertEqual(channel.code, 200, channel.result)
return channel.json_body["displayname"]
class ProfilesRestrictedTestCase(unittest.HomeserverTestCase): class ProfilesRestrictedTestCase(unittest.HomeserverTestCase):
servlets = [ servlets = [