From 0442e95276f7e4a0a7eeb83142eff4f855f0cf89 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Mon, 17 Jun 2024 19:21:45 +0000 Subject: [PATCH] emoji: Use a non-predictable filename. We use a truncated SHA256 of the id and a server-side secret to make emoji have non-guessable filenames, while also making collisions unlikely. We also adjust the Slack import to use the same SHA-based name, instead of taking the same name as it had in Slack. --- zerver/data_import/slack.py | 7 +++++-- zerver/lib/emoji.py | 15 ++++++++++++++- zerver/tests/test_message_notification_emails.py | 7 +++++-- zerver/tests/test_realm_emoji.py | 3 ++- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/zerver/data_import/slack.py b/zerver/data_import/slack.py index ddc123d8b0..193426448a 100644 --- a/zerver/data_import/slack.py +++ b/zerver/data_import/slack.py @@ -44,8 +44,9 @@ from zerver.data_import.slack_message_conversion import ( convert_to_zulip_markdown, get_user_full_name, ) -from zerver.lib.emoji import codepoint_to_name +from zerver.lib.emoji import codepoint_to_name, get_emoji_file_name from zerver.lib.export import MESSAGE_BATCH_CHUNK_SIZE +from zerver.lib.mime_types import guess_type from zerver.lib.storage import static_path from zerver.lib.thumbnail import resize_logo from zerver.lib.upload import sanitize_name @@ -220,10 +221,12 @@ def build_realmemoji( if split_url.hostname == "emoji.slack-edge.com": # Some of the emojis we get from the API have invalid links # this is to prevent errors related to them + content_type = guess_type(posixpath.basename(split_url.path))[0] + assert content_type is not None realmemoji = RealmEmoji( name=emoji_name, id=emoji_id, - file_name=posixpath.basename(split_url.path), + file_name=get_emoji_file_name(content_type, emoji_id), deactivated=False, ) diff --git a/zerver/lib/emoji.py b/zerver/lib/emoji.py index 0fd7f70c9c..0b13099c60 100644 --- a/zerver/lib/emoji.py +++ b/zerver/lib/emoji.py @@ -1,8 +1,10 @@ +import hashlib import os import re from dataclasses import dataclass import orjson +from django.conf import settings from django.contrib.staticfiles.storage import staticfiles_storage from django.utils.translation import gettext as _ @@ -154,4 +156,15 @@ def get_emoji_file_name(content_type: str, emoji_id: int) -> str: # The only callsite of this pre-limits the content_type to a # reasonable set that we know have extensions. assert image_ext is not None - return "".join((str(emoji_id), image_ext)) + + # We salt this with a server-side secret so that it is not + # enumerable by clients, and will not collide on the server. New + # realm imports may pass a synthetic emoji_id, which is fine as + # long as it starts at 1, and as such later emoji cannot collide + # unless there is a legit hash collision. + # + # We truncate the hash at 8 characters, as this is enough entropy + # to make collisions vanishingly unlikely. In the event of a + # collusion, the id will advance and a manual retry will succeed. + hash_key = settings.AVATAR_SALT.encode() + b":" + str(emoji_id).encode() + return "".join((hashlib.sha256(hash_key).hexdigest()[0:8], image_ext)) diff --git a/zerver/tests/test_message_notification_emails.py b/zerver/tests/test_message_notification_emails.py index e3c85c1dfc..94dab19336 100644 --- a/zerver/tests/test_message_notification_emails.py +++ b/zerver/tests/test_message_notification_emails.py @@ -25,6 +25,7 @@ from zerver.lib.email_notifications import ( include_realm_name_in_missedmessage_emails_subject, relative_to_full_url, ) +from zerver.lib.emoji import get_emoji_file_name from zerver.lib.send_email import FromAddress from zerver.lib.test_classes import ZulipTestCase from zerver.models import UserMessage, UserProfile, UserTopic @@ -1175,9 +1176,11 @@ class TestMessageNotificationEmails(ZulipTestCase): "Extremely personal message with a realm emoji :green_tick:!", ) realm_emoji_dict = get_name_keyed_dict_for_active_realm_emoji(realm.id) - realm_emoji_id = realm_emoji_dict["green_tick"]["id"] + realm_emoji_file_name = get_emoji_file_name( + "image/png", int(realm_emoji_dict["green_tick"]["id"]) + ) realm_emoji_url = ( - f"http://zulip.testserver/user_avatars/{realm.id}/emoji/images/{realm_emoji_id}.png" + f"http://zulip.testserver/user_avatars/{realm.id}/emoji/images/{realm_emoji_file_name}" ) verify_body_include = [ f':green_tick:' diff --git a/zerver/tests/test_realm_emoji.py b/zerver/tests/test_realm_emoji.py index 3f8c613bae..7f757c65d4 100644 --- a/zerver/tests/test_realm_emoji.py +++ b/zerver/tests/test_realm_emoji.py @@ -5,6 +5,7 @@ from zerver.actions.create_user import do_create_user from zerver.actions.realm_emoji import check_add_realm_emoji from zerver.actions.realm_settings import do_set_realm_property from zerver.actions.users import do_change_user_role +from zerver.lib.emoji import get_emoji_file_name from zerver.lib.exceptions import JsonableError from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import get_test_image_file @@ -112,7 +113,7 @@ class RealmEmojiTest(ZulipTestCase): def test_realm_emoji_repr(self) -> None: realm_emoji = RealmEmoji.objects.get(name="green_tick") - file_name = str(realm_emoji.id) + ".png" + file_name = get_emoji_file_name("image/png", realm_emoji.id) self.assertEqual( repr(realm_emoji), f"",