CVE-2025-25195: Only send "active" change events to channel subscribers.

This fixes a bug where private stream event to update stream's
active status was sent to all active users instead of just
its subscribers.
This commit is contained in:
Aman Agrawal
2025-01-30 13:32:08 +05:30
committed by Tim Abbott
parent a2a1a7f8d1
commit 75be449d45
2 changed files with 87 additions and 4 deletions

View File

@@ -1550,7 +1550,7 @@ def notify_stream_is_recently_active_update(stream: Stream, value: bool) -> None
name=stream.name, name=stream.name,
) )
send_event_on_commit(stream.realm, event, active_user_ids(stream.realm_id)) send_event_on_commit(stream.realm, event, can_access_stream_metadata_user_ids(stream))
@transaction.atomic(durable=True) @transaction.atomic(durable=True)

View File

@@ -1,16 +1,21 @@
from datetime import timedelta from datetime import timedelta
from unittest import mock
import orjson import orjson
from django.utils.timezone import now as timezone_now
from zerver.actions.message_delete import do_delete_messages from zerver.actions.message_delete import do_delete_messages
from zerver.actions.realm_settings import ( from zerver.actions.realm_settings import (
do_change_realm_permission_group_setting, do_change_realm_permission_group_setting,
do_set_realm_property, do_set_realm_property,
) )
from zerver.actions.streams import do_change_stream_group_based_setting from zerver.actions.streams import do_change_stream_group_based_setting, do_change_stream_permission
from zerver.actions.user_groups import check_add_user_group from zerver.actions.user_groups import check_add_user_group
from zerver.lib.message import has_message_access from zerver.lib.message import has_message_access
from zerver.lib.streams import check_update_all_streams_active_status from zerver.lib.streams import (
can_access_stream_metadata_user_ids,
update_stream_active_status_for_realm,
)
from zerver.lib.test_classes import ZulipTestCase, get_topic_messages from zerver.lib.test_classes import ZulipTestCase, get_topic_messages
from zerver.lib.test_helpers import queries_captured from zerver.lib.test_helpers import queries_captured
from zerver.lib.url_encoding import near_stream_message_url from zerver.lib.url_encoding import near_stream_message_url
@@ -18,6 +23,7 @@ from zerver.models import Message, NamedUserGroup, Stream, UserMessage, UserProf
from zerver.models.groups import SystemGroups from zerver.models.groups import SystemGroups
from zerver.models.realms import get_realm from zerver.models.realms import get_realm
from zerver.models.streams import get_stream from zerver.models.streams import get_stream
from zerver.tornado.django_api import send_event_on_commit
class MessageMoveStreamTest(ZulipTestCase): class MessageMoveStreamTest(ZulipTestCase):
@@ -1932,7 +1938,84 @@ class MessageMoveStreamTest(ZulipTestCase):
# Delete all messages in new stream and mark it as inactive. # Delete all messages in new stream and mark it as inactive.
Message.objects.filter(recipient__type_id=new_stream.id, realm=user_profile.realm).delete() Message.objects.filter(recipient__type_id=new_stream.id, realm=user_profile.realm).delete()
check_update_all_streams_active_status() with mock.patch("zerver.lib.streams.send_event_on_commit", wraps=send_event_on_commit) as m:
update_stream_active_status_for_realm(
user_profile.realm, timezone_now() - timedelta(days=10)
)
self.assertEqual(
m.call_args.args,
(
new_stream.realm,
dict(
type="stream",
op="update",
property="is_recently_active",
value=False,
stream_id=new_stream.id,
name=new_stream.name,
),
can_access_stream_metadata_user_ids(new_stream),
),
)
new_stream.refresh_from_db()
self.assertFalse(new_stream.is_recently_active)
# Move the message to new stream should make active again.
result = self.client_patch(
f"/json/messages/{msg_id_later}",
{
"stream_id": new_stream.id,
"propagate_mode": "change_later",
"send_notification_to_new_thread": "false",
},
)
self.assert_json_success(result)
new_stream.refresh_from_db()
self.assertTrue(new_stream.is_recently_active)
def test_move_message_update_private_stream_active_status(self) -> None:
# Goal is to test that we only send the stream status update to subscribers.
(user_profile, old_stream, new_stream, msg_id, msg_id_later) = self.prepare_move_topics(
"iago", "test move stream", "new stream", "test"
)
# Mark stream as private
do_change_stream_permission(
new_stream,
invite_only=True,
history_public_to_subscribers=False,
is_web_public=False,
acting_user=user_profile,
)
# Delete all messages in new stream and mark it as inactive.
Message.objects.filter(recipient__type_id=new_stream.id, realm=user_profile.realm).delete()
with mock.patch("zerver.lib.streams.send_event_on_commit", wraps=send_event_on_commit) as m:
update_stream_active_status_for_realm(
user_profile.realm, timezone_now() - timedelta(days=10)
)
self.assertEqual(
m.call_args.args,
(
new_stream.realm,
dict(
type="stream",
op="update",
property="is_recently_active",
value=False,
stream_id=new_stream.id,
name=new_stream.name,
),
# Only send the event to users with stream access.
{
9, # Realm owner (Desdemona)
11, # Subscriber (iago)
},
),
)
new_stream.refresh_from_db() new_stream.refresh_from_db()
self.assertFalse(new_stream.is_recently_active) self.assertFalse(new_stream.is_recently_active)