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

Remove any NULL characters from remote displaynames before updating user directory #12743

Closed
Closed
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
2 changes: 2 additions & 0 deletions changelog.d/12743.bugfix
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions synapse/handlers/user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions synapse/storage/databases/main/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down
7 changes: 3 additions & 4 deletions synapse/storage/databases/main/user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 6 additions & 0 deletions synapse/storage/util/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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