CVE-2016-4426: Fix non-admin users having access to all bot API keys.

Long ago, there was work on an experimental integration model where
every user in a realm would have administrative control over all bots,
with the goal of simplifying the process of setting up communally
administered bots for smaller teams.  While that new model was never
fully implemented (and thus never setup as an option), an error in
that original implementation meant that the data on all bots in a
realm, including their API keys, was sent to the browsers of users via
the `realm_bots` variable in `page_params`.  The data wasn't displayed
in the UI for non-admin users, but was available via e.g. the
javascript console.

This commit updates this behavior to only send sensitive bot data like
API keys to the owner of the bot (and realm admins).

We may in the future implement a model simplifying communally
administered integrations, but if we do that, those bots should be
limited in their capabilities (e.g. only able to send webhook
messages).

This bug has been present since Zulip was released as open source.

zerver/lib/actions.py

@@ -17,7 +17,7 @@ from zerver.models import Realm, RealmEmoji, Stream, UserProfile, UserActivity,
     get_user_profile_by_email, get_stream_cache_key, to_dict_cache_key_id, \
     UserActivityInterval, get_active_user_dicts_in_realm, get_active_streams, \
     realm_filters_for_domain, RealmFilter, receives_offline_notifications, \
-    ScheduledJob, realm_filters_for_domain, get_active_bot_dicts_in_realm
+    ScheduledJob, realm_filters_for_domain, get_owned_bot_dicts
 
 from zerver.lib.avatar import get_avatar_url, avatar_url
 
@@ -2432,18 +2432,6 @@ def get_realm_user_dicts(user_profile):
              'full_name' : userdict['full_name']}
             for userdict in get_active_user_dicts_in_realm(user_profile.realm)]
 
-def get_realm_bot_dicts(user_profile):
-    return [{'email': botdict['email'],
-             'full_name': botdict['full_name'],
-             'api_key': botdict['api_key'],
-             'default_sending_stream': botdict['default_sending_stream__name'],
-             'default_events_register_stream': botdict['default_events_register_stream__name'],
-             'default_all_public_streams': botdict['default_all_public_streams'],
-             'owner': botdict['bot_owner__email'],
-             'avatar_url': get_avatar_url(botdict['avatar_source'], botdict['email']),
-            }
-            for botdict in get_active_bot_dicts_in_realm(user_profile.realm)]
-
 # Fetch initial data.  When event_types is not specified, clients want
 # all event types.  Whenever you add new code to this function, you
 # should also add corresponding events for changes in the data
@@ -2497,7 +2485,7 @@ def fetch_initial_state_data(user_profile, event_types, queue_id):
         state['realm_users'] = get_realm_user_dicts(user_profile)
 
     if want('realm_bot'):
-        state['realm_bots'] = get_realm_bot_dicts(user_profile)
+        state['realm_bots'] = get_owned_bot_dicts(user_profile)
 
     if want('referral'):
         state['referrals'] = {'granted': user_profile.invites_granted,
@@ -2542,8 +2530,20 @@ def apply_events(state, events, user_profile):
             elif event['op'] == 'update':
                 for p in state['realm_users']:
                     if our_person(p):
+                        # In the unlikely event that the current user
+                        # just changed to/from being an admin, we need
+                        # to add/remove the data on all bots in the
+                        # realm.  This is ugly and probably better
+                        # solved by removing the all-realm-bots data
+                        # given to admin users from this flow.
+                        if ('is_admin' in person and 'realm_bots' in state and
+                            user_profile.email == person['email']):
+                            if p['is_admin'] and not person['is_admin']:
+                                state['realm_bots'] = []
+                            if not p['is_admin'] and person['is_admin']:
+                                state['realm_bots'] = get_owned_bot_dicts(user_profile)
+                        # Now update the person
                         p.update(person)
-
         elif event['type'] == 'realm_bot':
             if event['op'] == 'add':
                 state['realm_bots'].append(event['bot'])

zerver/models.py

@@ -1109,6 +1109,23 @@ def get_active_bot_dicts_in_realm(realm):
     return UserProfile.objects.filter(realm=realm, is_active=True, is_bot=True) \
                               .values(*active_bot_dict_fields)
 
+def get_owned_bot_dicts(user_profile, include_all_realm_bots_if_admin=True):
+    if user_profile.is_realm_admin and include_all_realm_bots_if_admin:
+        result = get_active_bot_dicts_in_realm(user_profile.realm)
+    else:
+        result = UserProfile.objects.filter(realm=user_profile.realm, is_active=True, is_bot=True,
+                                        bot_owner=user_profile).values(*active_bot_dict_fields)
+    return [{'email': botdict['email'],
+             'full_name': botdict['full_name'],
+             'api_key': botdict['api_key'],
+             'default_sending_stream': botdict['default_sending_stream__name'],
+             'default_events_register_stream': botdict['default_events_register_stream__name'],
+             'default_all_public_streams': botdict['default_all_public_streams'],
+             'owner': botdict['bot_owner__email'],
+             'avatar_url': get_avatar_url(botdict['avatar_source'], botdict['email']),
+            }
+            for botdict in result]
+
 def get_prereg_user_by_email(email):
     # A user can be invited many times, so only return the result of the latest
     # invite.