CVE-2022-31168: Improper Authorization (CWE-285)
value: Zulip is an open source team chat tool. Due to an incorrect authorization check in Zulip Server 5.4 and earlier, a member of an organization could craft an API call that grants organization administrator privileges to one of their bots. The vulnerability is fixed in Zulip Server 5.5. Members who don’t own any bots, and lack permission to create them, can’t exploit the vulnerability. As a workaround for the vulnerability, an organization administrator can restrict the `Who can create bots` permission to administrators only, and change the ownership of existing bots.
CWE description: The product does not perform or incorrectly performs an authorization check when an actor attempts to access a resource or perform an action.
CVSS v3 base score: 8.8
Published: 2022-07-22T13:15Z
Repository: https://github.com/zulip/zulip
Language: Python
Files changed: zerver/tests/test_bots.py, zerver/views/users.py
Commit date: 2022-07-21 17:59:09 -0700
Commit message: CVE-2022-31168: Fix authorization check for changing bot roles.

Due to an incorrect authorization check in Zulip Server 5.4 and
earlier, a member of an organization could craft an API call that
grants organization administrator privileges to one of their bots.

Signed-off-by: Anders Kaseorg <anders@zulip.com>

Diff: diff --git a/zerver/tests/test_bots.py b/zerver/tests/test_bots.py
index 2c10e7d6495..9004eacee93 100644
--- a/zerver/tests/test_bots.py
+++ b/zerver/tests/test_bots.py
@@ -6,10 +6,11 @@ from unittest.mock import MagicMock, patch
 import orjson
 from django.core import mail
 from django.test import override_settings
 from zulip_bots.custom_exceptions import ConfigValidationError
 
+from zerver.actions.bots import do_change_bot_owner
 from zerver.actions.realm_settings import do_set_realm_property
 from zerver.actions.streams import do_change_stream_permission
 from zerver.actions.users import do_change_can_create_users, do_change_user_role, do_deactivate_user
 from zerver.lib.bot_config import ConfigError, get_bot_config
 from zerver.lib.bot_lib import get_bot_handler
@@ -1188,13 +1189,30 @@ class BotTest(ZulipTestCase, UploadSerializeMixin):
         desdemona = self.example_user("desdemona")
         do_change_user_role(desdemona, UserProfile.ROLE_REALM_ADMINISTRATOR, acting_user=None)
 
         req = dict(role=UserProfile.ROLE_REALM_OWNER)
 
-        result = self.client_patch(f"/json/bots/{self.get_bot_user(email).id}", req)
+        result = self.client_patch(f"/json/users/{user_profile.id}", req)
+        self.assert_json_error(result, "Must be an organization owner")
+
+        result = self.client_patch(f"/json/bots/{user_profile.id}", req)
         self.assert_json_error(result, "Must be an organization owner")
 
+        # Test for not allowing a non-administrator user to assign a bot an administrator role
+        shiva = self.example_user("shiva")
+        self.assertEqual(shiva.role, UserProfile.ROLE_MODERATOR)
+        self.login_user(shiva)
+        do_change_bot_owner(user_profile, shiva, acting_user=None)
+
+        req = dict(role=UserProfile.ROLE_REALM_ADMINISTRATOR)
+
+        result = self.client_patch(f"/json/users/{user_profile.id}", req)
+        self.assert_json_error(result, "Must be an organization administrator")
+
+        result = self.client_patch(f"/json/bots/{user_profile.id}", req)
+        self.assert_json_error(result, "Must be an organization administrator")
+
     def test_patch_bot_to_stream_private_allowed(self) -> None:
         self.login("hamlet")
         user_profile = self.example_user("hamlet")
         stream = self.subscribe(user_profile, "Denmark")
         do_change_stream_permission(stream, invite_only=True, acting_user=user_profile)
diff --git a/zerver/views/users.py b/zerver/views/users.py
index f80ba7a6e3a..1fae6f40e16 100644
--- a/zerver/views/users.py
+++ b/zerver/views/users.py
@@ -38,10 +38,11 @@ from zerver.lib.bot_config import set_bot_config
 from zerver.lib.email_validation import email_allowed_for_realm
 from zerver.lib.exceptions import (
     CannotDeactivateLastUserError,
     JsonableError,
     MissingAuthenticationError,
+    OrganizationAdministratorRequired,
     OrganizationOwnerRequired,
     RateLimited,
 )
 from zerver.lib.integrations import EMBEDDED_BOTS
 from zerver.lib.rate_limiter import rate_limit_spectator_attachment_access_by_file
@@ -189,17 +190,17 @@ def update_user_backend(
         user_profile, user_id, allow_deactivated=True, allow_bots=True, for_admin=True
     )
 
     if role is not None and target.role != role:
         # Require that the current user has permissions to
-        # grant/remove the role in question.  access_user_by_id has
-        # already verified we're an administrator; here we enforce
-        # that only owners can toggle the is_realm_owner flag.
+        # grant/remove the role in question.
         #
         # Logic replicated in patch_bot_backend.
         if UserProfile.ROLE_REALM_OWNER in [role, target.role] and not user_profile.is_realm_owner:
             raise OrganizationOwnerRequired()
+        elif not user_profile.is_realm_admin:
+            raise OrganizationAdministratorRequired()
 
         if target.role == UserProfile.ROLE_REALM_OWNER and check_last_owner(target):
             raise JsonableError(
                 _("The owner permission cannot be removed from the only organization owner.")
             )
@@ -329,10 +330,12 @@ def patch_bot_backend(
 
     if role is not None and bot.role != role:
         # Logic duplicated from update_user_backend.
         if UserProfile.ROLE_REALM_OWNER in [role, bot.role] and not user_profile.is_realm_owner:
             raise OrganizationOwnerRequired()
+        elif not user_profile.is_realm_admin:
+            raise OrganizationAdministratorRequired()
 
         do_change_user_role(bot, role, acting_user=user_profile)
 
     if bot_owner_id is not None:
         try:

Vulnerable code:         result = self.client_patch(f"/json/bots/{self.get_bot_user(email).id}", req)
        # grant/remove the role in question.  access_user_by_id has
        # already verified we're an administrator; here we enforce
        # that only owners can toggle the is_realm_owner flag.
Fixed code: from zerver.actions.bots import do_change_bot_owner
        result = self.client_patch(f"/json/users/{user_profile.id}", req)
        self.assert_json_error(result, "Must be an organization owner")

        result = self.client_patch(f"/json/bots/{user_profile.id}", req)
        # Test for not allowing a non-administrator user to assign a bot an administrator role
        shiva = self.example_user("shiva")
        self.assertEqual(shiva.role, UserProfile.ROLE_MODERATOR)
        self.login_user(shiva)
        do_change_bot_owner(user_profile, shiva, acting_user=None)

        req = dict(role=UserProfile.ROLE_REALM_ADMINISTRATOR)

        result = self.client_patch(f"/json/users/{user_profile.id}", req)
        self.assert_json_error(result, "Must be an organization administrator")

        result = self.client_patch(f"/json/bots/{user_profile.id}", req)
        self.assert_json_error(result, "Must be an organization administrator")

    OrganizationAdministratorRequired,
        # grant/remove the role in question.
        elif not user_profile.is_realm_admin:
            raise OrganizationAdministratorRequired()
        elif not user_profile.is_realm_admin:
            raise OrganizationAdministratorRequired()