diff --git a/changelog.d/12743.bugfix b/changelog.d/12743.bugfix new file mode 100644 index 000000000000..1227e583ca16 --- /dev/null +++ b/changelog.d/12743.bugfix @@ -0,0 +1,2 @@ +Fix a long-standing bug wherein remote display names or avatar URLs containing null bytes caused updating the user directory to fail resulting in high CPU on the main process. Contributed +by Nick @ Beeper. diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py index 74f7fdfe6ce5..ea03ac959722 100644 --- a/synapse/handlers/user_directory.py +++ b/synapse/handlers/user_directory.py @@ -464,6 +464,7 @@ async def _handle_possible_remote_profile_change( 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 diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 42d484dc98d9..c2ea2d199397 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -47,6 +47,7 @@ from synapse.storage.databases.main.events_worker import EventCacheEntry from synapse.storage.databases.main.search import SearchEntry from synapse.storage.engines.postgres import PostgresEngine +from synapse.storage.util import non_null_str_or_none from synapse.storage.util.id_generators import AbstractStreamIdGenerator from synapse.storage.util.sequence import SequenceGenerator from synapse.types import JsonDict, StateMap, get_domain_from_id @@ -1728,9 +1729,6 @@ def _store_room_members_txn( not affect the current local state. """ - def non_null_str_or_none(val: Any) -> Optional[str]: - return val if isinstance(val, str) and "\u0000" not in val else None - self.db_pool.simple_insert_many_txn( txn, table="room_memberships", diff --git a/synapse/storage/databases/main/user_directory.py b/synapse/storage/databases/main/user_directory.py index df772d472102..59b86e6cd457 100644 --- a/synapse/storage/databases/main/user_directory.py +++ b/synapse/storage/databases/main/user_directory.py @@ -42,6 +42,7 @@ from synapse.storage.databases.main.state import StateFilter from synapse.storage.databases.main.state_deltas import StateDeltasStore from synapse.storage.engines import PostgresEngine, Sqlite3Engine +from synapse.storage.util import non_null_str_or_none from synapse.types import ( JsonDict, UserProfile, @@ -470,10 +471,8 @@ 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 + display_name = non_null_str_or_none(display_name) + avatar_url = non_null_str_or_none(avatar_url) def _update_profile_in_user_dir_txn(txn: LoggingTransaction) -> None: self.db_pool.simple_upsert_txn( diff --git a/synapse/storage/util/__init__.py b/synapse/storage/util/__init__.py index 5e83dba2ed6f..04d5b42911b6 100644 --- a/synapse/storage/util/__init__.py +++ b/synapse/storage/util/__init__.py @@ -11,3 +11,9 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + +from typing import Any, Optional + + +def non_null_str_or_none(val: Any) -> Optional[str]: + return val if isinstance(val, str) and "\u0000" not in val else None