Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Fix errors when updating the user directory with invalid data (#8223)
Browse files Browse the repository at this point in the history
  • Loading branch information
clokep authored Sep 1, 2020
1 parent b5133dd commit b939251
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 1 deletion.
1 change: 1 addition & 0 deletions changelog.d/8223.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes a longstanding bug where user directory updates could break when unexpected profile data was included in events.
6 changes: 6 additions & 0 deletions synapse/handlers/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,9 @@ async def set_displayname(
Codes.FORBIDDEN,
)

if not isinstance(new_displayname, str):
raise SynapseError(400, "Invalid displayname")

if len(new_displayname) > MAX_DISPLAYNAME_LEN:
raise SynapseError(
400, "Displayname is too long (max %i)" % (MAX_DISPLAYNAME_LEN,)
Expand Down Expand Up @@ -235,6 +238,9 @@ async def set_avatar_url(
400, "Changing avatar is disabled on this server", Codes.FORBIDDEN
)

if not isinstance(new_avatar_url, str):
raise SynapseError(400, "Invalid displayname")

if len(new_avatar_url) > MAX_AVATAR_URL_LEN:
raise SynapseError(
400, "Avatar URL is too long (max %i)" % (MAX_AVATAR_URL_LEN,)
Expand Down
8 changes: 7 additions & 1 deletion synapse/handlers/user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ async def _handle_deltas(self, deltas):
async def _handle_room_publicity_change(
self, room_id, prev_event_id, event_id, typ
):
"""Handle a room having potentially changed from/to world_readable/publically
"""Handle a room having potentially changed from/to world_readable/publicly
joinable.
Args:
Expand Down Expand Up @@ -388,9 +388,15 @@ async def _handle_profile_change(self, user_id, room_id, prev_event_id, event_id

prev_name = prev_event.content.get("displayname")
new_name = event.content.get("displayname")
# If the new name is an unexpected form, do not update the directory.
if not isinstance(new_name, str):
new_name = prev_name

prev_avatar = prev_event.content.get("avatar_url")
new_avatar = event.content.get("avatar_url")
# If the new avatar is an unexpected form, do not update the directory.
if not isinstance(new_avatar, str):
new_avatar = prev_avatar

if prev_name != new_name or prev_avatar != new_avatar:
await self.store.update_profile_in_user_dir(user_id, new_name, new_avatar)
5 changes: 5 additions & 0 deletions synapse/storage/databases/main/user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,11 @@ async def update_profile_in_user_dir(
"""
Update or add a user's profile in the user directory.
"""
# If the display name or avatar URL are unexpected types, overwrite them.
if not isinstance(display_name, str):
display_name = None
if not isinstance(avatar_url, str):
avatar_url = None

def _update_profile_in_user_dir_txn(txn):
new_entry = self.db_pool.simple_upsert_txn(
Expand Down

0 comments on commit b939251

Please sign in to comment.