paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-11-01 12:33:40 +00:00
Code Issues Packages Projects Releases Wiki Activity

attachments: Restrict users access to attachment without message access.

Browse Source 
This commit adds hardening such that if the invariant "no usermessage
row corresponding to a message exists if the user loses access to the
message" is violated due to some bug, user can't access the attachment
if they are not subscribed.
This commit is contained in:
Prakhar Pratyush
2024-12-23 20:38:10 +05:30
committed by Tim Abbott
parent 621eb1f610
commit ab123d3160
2 changed files with 29 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
zerver/lib/attachments.py
View File

@@ -132,28 +132,43 @@ def validate_attachment_request(
return True, attachment
messages = attachment.messages.all()
if UserMessage.objects.filter(user_profile=user_profile, message__in=messages).exists():
# If it was sent in a direct message or private stream
# message, then anyone who received that message can access it.
return True, attachment
# The user didn't receive any of the messages that included this
# attachment. But they might still have access to it, if it was
# sent to a stream they are on where history is public to
# subscribers.
usermessages_channel_ids = set()
usermessage_rows = UserMessage.objects.filter(
user_profile=user_profile, message__in=messages
).select_related("message", "message__recipient")
for um in usermessage_rows:
if not um.message.is_stream_message():
# If the attachment was sent in a direct message or group direct
# message then anyone who received that message can access it.
return True, attachment
else:
usermessages_channel_ids.add(um.message.recipient.type_id)
# These are subscriptions to a stream one of the messages was sent to
relevant_stream_ids = Subscription.objects.filter(
# These are subscriptions to a channel one of the messages was sent to
relevant_channel_ids = Subscription.objects.filter(
user_profile=user_profile,
active=True,
recipient__type=Recipient.STREAM,
recipient__in=[m.recipient_id for m in messages],
).values_list("recipient__type_id", flat=True)
if len(relevant_stream_ids) == 0:
if usermessages_channel_ids & set(relevant_channel_ids):
# If the attachment was sent in a channel with public
# or protected history and the user is still subscribed
# to the channel then anyone who received that message
# can access it.
return True, attachment
# The user didn't receive any of the messages that included this
# attachment. But they might still have access to it, if it was
# sent to a stream they are on where history is public to
# subscribers.
if len(relevant_channel_ids) == 0:
return False, attachment
return Stream.objects.filter(
id__in=relevant_stream_ids, history_public_to_subscribers=True
id__in=relevant_channel_ids, history_public_to_subscribers=True
).exists(), attachment

4
zerver/tests/test_upload.py
View File

@@ -809,7 +809,7 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase):
# Subscribed user who received the message should be able to view file
self.login_user(cordelia)
with self.assert_database_query_count(7):
with self.assert_database_query_count(9):
response = self.client_get(url)
self.assertEqual(response.status_code, 200)
self.assertEqual(response.getvalue(), b"zulip!")
@@ -870,7 +870,7 @@ class FileUploadTest(UploadSerializeMixin, ZulipTestCase):
# Originally subscribed user should be able to view file
self.login_user(polonius)
with self.assert_database_query_count(7):
with self.assert_database_query_count(9):
response = self.client_get(url)
self.assertEqual(response.status_code, 200)
self.assertEqual(response.getvalue(), b"zulip!")