CVE-2021-30477: Prevent outgoing webhook bots from sending arbitrary messages to any stream.

A bug in the implementation of replies to messages sent by outgoing
webhooks to private streams meant that an outgoing webhook bot could be
used to send messages to private streams that the user was not intended
to be able to send messages to.

Completely skipping stream access check in check_message whenever the
sender is an outgoing webhook bot is insecure, as it might allow someone
with access to the bot's API key to send arbitrary messages to all
streams in the organization. The check is only meant to be bypassed in
send_response_message, where the stream message is only being sent
because someone mentioned the bot in that stream (and thus the bot
posting there is the desired outcome). We get much better control over
what's going by passing an explicit argument to check_message when
skipping the access check is desirable.

zerver/lib/actions.py

@@ -2541,6 +2541,8 @@ def check_send_message(
     local_id: Optional[str] = None,
     sender_queue_id: Optional[str] = None,
     widget_content: Optional[str] = None,
+    *,
+    skip_stream_access_check: bool = False,
 ) -> int:
 
     addressee = Addressee.legacy_build(sender, message_type_name, message_to, topic_name)
@@ -2557,6 +2559,7 @@ def check_send_message(
             local_id,
             sender_queue_id,
             widget_content,
+            skip_stream_access_check=skip_stream_access_check,
         )
     except ZephyrMessageAlreadySentException as e:
         return e.message_id
@@ -2754,6 +2757,8 @@ def check_message(
     sender_queue_id: Optional[str] = None,
     widget_content: Optional[str] = None,
     email_gateway: bool = False,
+    *,
+    skip_stream_access_check: bool = False,
 ) -> SendMessageRequest:
     """See
     https://zulip.readthedocs.io/en/latest/subsystems/sending-messages.html
@@ -2796,10 +2801,16 @@ def check_message(
             type=Recipient.STREAM,
         )
 
-        if sender.bot_type != sender.OUTGOING_WEBHOOK_BOT:
+        if not skip_stream_access_check:
             access_stream_for_send_message(
                 sender=sender, stream=stream, forwarder_user_profile=forwarder_user_profile
             )
+        else:
+            # Defensive assertion - the only currently supported use case
+            # for this option is for outgoing webhook bots and since this
+            # is security-sensitive code, it's beneficial to ensure nothing
+            # else can sneak past the access check.
+            assert sender.bot_type == sender.OUTGOING_WEBHOOK_BOT
 
     elif addressee.is_private():
         user_profiles = addressee.user_profiles()

zerver/lib/outgoing_webhook.py

@@ -162,6 +162,10 @@ def send_response_message(
 
     response_data is what the bot wants to send back and has these fields:
         content - raw Markdown content for Zulip to render
+
+    WARNING: This function sends messages bypassing the stream access check
+    for the bot - so use with caution to not call this in codepaths
+    that might let someone send arbitrary messages to any stream through this.
     """
 
     message_type = message_info["type"]
@@ -196,6 +200,7 @@ def send_response_message(
         message_content=content,
         widget_content=widget_content,
         realm=realm,
+        skip_stream_access_check=True,
     )