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

Fix errors when updating the user directory with invalid data #8223

Merged
merged 4 commits into from
Sep 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -369,6 +369,11 @@ def update_profile_in_user_dir(self, user_id, display_name, avatar_url):
"""
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