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.
This commit is contained in:
Alex Vandiver 2024-06-17 19:21:45 +00:00 committed by Tim Abbott
parent 49c0f7306e
commit 0442e95276
4 changed files with 26 additions and 6 deletions

View File

@ -44,8 +44,9 @@ from zerver.data_import.slack_message_conversion import (
convert_to_zulip_markdown, convert_to_zulip_markdown,
get_user_full_name, 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.export import MESSAGE_BATCH_CHUNK_SIZE
from zerver.lib.mime_types import guess_type
from zerver.lib.storage import static_path from zerver.lib.storage import static_path
from zerver.lib.thumbnail import resize_logo from zerver.lib.thumbnail import resize_logo
from zerver.lib.upload import sanitize_name from zerver.lib.upload import sanitize_name
@ -220,10 +221,12 @@ def build_realmemoji(
if split_url.hostname == "emoji.slack-edge.com": if split_url.hostname == "emoji.slack-edge.com":
# Some of the emojis we get from the API have invalid links # Some of the emojis we get from the API have invalid links
# this is to prevent errors related to them # 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( realmemoji = RealmEmoji(
name=emoji_name, name=emoji_name,
id=emoji_id, id=emoji_id,
file_name=posixpath.basename(split_url.path), file_name=get_emoji_file_name(content_type, emoji_id),
deactivated=False, deactivated=False,
) )

View File

@ -1,8 +1,10 @@
import hashlib
import os import os
import re import re
from dataclasses import dataclass from dataclasses import dataclass
import orjson import orjson
from django.conf import settings
from django.contrib.staticfiles.storage import staticfiles_storage from django.contrib.staticfiles.storage import staticfiles_storage
from django.utils.translation import gettext as _ 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 # The only callsite of this pre-limits the content_type to a
# reasonable set that we know have extensions. # reasonable set that we know have extensions.
assert image_ext is not None 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))

View File

@ -25,6 +25,7 @@ from zerver.lib.email_notifications import (
include_realm_name_in_missedmessage_emails_subject, include_realm_name_in_missedmessage_emails_subject,
relative_to_full_url, relative_to_full_url,
) )
from zerver.lib.emoji import get_emoji_file_name
from zerver.lib.send_email import FromAddress from zerver.lib.send_email import FromAddress
from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_classes import ZulipTestCase
from zerver.models import UserMessage, UserProfile, UserTopic from zerver.models import UserMessage, UserProfile, UserTopic
@ -1175,9 +1176,11 @@ class TestMessageNotificationEmails(ZulipTestCase):
"Extremely personal message with a realm emoji :green_tick:!", "Extremely personal message with a realm emoji :green_tick:!",
) )
realm_emoji_dict = get_name_keyed_dict_for_active_realm_emoji(realm.id) 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 = ( 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 = [ verify_body_include = [
f'<img alt=":green_tick:" src="{realm_emoji_url}" title="green tick" style="height: 20px;">' f'<img alt=":green_tick:" src="{realm_emoji_url}" title="green tick" style="height: 20px;">'

View File

@ -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_emoji import check_add_realm_emoji
from zerver.actions.realm_settings import do_set_realm_property from zerver.actions.realm_settings import do_set_realm_property
from zerver.actions.users import do_change_user_role 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.exceptions import JsonableError
from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_classes import ZulipTestCase
from zerver.lib.test_helpers import get_test_image_file from zerver.lib.test_helpers import get_test_image_file
@ -112,7 +113,7 @@ class RealmEmojiTest(ZulipTestCase):
def test_realm_emoji_repr(self) -> None: def test_realm_emoji_repr(self) -> None:
realm_emoji = RealmEmoji.objects.get(name="green_tick") 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( self.assertEqual(
repr(realm_emoji), repr(realm_emoji),
f"<RealmEmoji: zulip: {realm_emoji.id} green_tick False {file_name}>", f"<RealmEmoji: zulip: {realm_emoji.id} green_tick False {file_name}>",