CVE-2020-14215: Fix validation in PreregistrationUser queries.

The most import change here is the one in maybe_send_to_registration
codepath, as the insufficient validation there could lead to fetching
an expired PreregistrationUser that was invited as an administrator
admin even years ago, leading to this registration ending up in the
new user being a realm administrator.

Combined with the buggy migration in
0198_preregistrationuser_invited_as.py, this led to users incorrectly
joining as organizations administrators by accident.  But even without
that bug, this issue could have allowed a user who was invited as an
administrator but then had that invitation expire and then joined via
social authentication incorrectly join as an organization administrator.

The second change is in ConfirmationEmailWorker, where this wasn't a
security problem, but if the server was stopped for long enough, with
some invites to send out email for in the queue, then after starting it
up again, the queue worker would send out emails for invites that
had already expired.

zerver/lib/actions.py

@@ -200,6 +200,7 @@ from zerver.models import (
     active_user_ids,
     custom_profile_fields_for_realm,
     email_to_username,
+    filter_to_valid_prereg_users,
     get_active_streams,
     get_bot_dicts_in_realm,
     get_bot_services,
@@ -5129,17 +5130,14 @@ def do_invite_users(user_profile: UserProfile,
     notify_invites_changed(user_profile)
 
 def do_get_user_invites(user_profile: UserProfile) -> List[Dict[str, Any]]:
-    days_to_activate = settings.INVITATION_LINK_VALIDITY_DAYS
-    active_value = confirmation_settings.STATUS_ACTIVE
-    revoked_value = confirmation_settings.STATUS_REVOKED
-    lowest_datetime = timezone_now() - datetime.timedelta(days=days_to_activate)
-    base_query = PreregistrationUser.objects.exclude(status__in=[active_value, revoked_value]).filter(
-        invited_at__gte=lowest_datetime)
-
     if user_profile.is_realm_admin:
-        prereg_users = base_query.filter(referred_by__realm=user_profile.realm)
+        prereg_users = filter_to_valid_prereg_users(
+            PreregistrationUser.objects.filter(referred_by__realm=user_profile.realm)
+        )
     else:
-        prereg_users = base_query.filter(referred_by=user_profile)
+        prereg_users = filter_to_valid_prereg_users(
+            PreregistrationUser.objects.filter(referred_by=user_profile)
+        )
 
     invites = []
 
@@ -5151,6 +5149,7 @@ def do_get_user_invites(user_profile: UserProfile) -> List[Dict[str, Any]]:
                             invited_as=invitee.invited_as,
                             is_multiuse=False))
 
+    lowest_datetime = timezone_now() - datetime.timedelta(days=settings.INVITATION_LINK_VALIDITY_DAYS)
     multiuse_confirmation_objs = Confirmation.objects.filter(realm=user_profile.realm,
                                                              type=Confirmation.MULTIUSE_INVITE,
                                                              date_sent__gte=lowest_datetime)

zerver/models.py

@@ -35,6 +35,7 @@ from django.db.models.signals import post_delete, post_save
 from django.utils.timezone import now as timezone_now
 from django.utils.translation import ugettext_lazy as _
 
+from confirmation import settings as confirmation_settings
 from zerver.lib import cache
 from zerver.lib.cache import (
     active_non_guest_user_ids_cache_key,
@@ -1357,6 +1358,14 @@ class PreregistrationUser(models.Model):
     )
     invited_as: int = models.PositiveSmallIntegerField(default=INVITE_AS['MEMBER'])
 
+def filter_to_valid_prereg_users(query: QuerySet) -> QuerySet:
+    days_to_activate = settings.INVITATION_LINK_VALIDITY_DAYS
+    active_value = confirmation_settings.STATUS_ACTIVE
+    revoked_value = confirmation_settings.STATUS_REVOKED
+    lowest_datetime = timezone_now() - datetime.timedelta(days=days_to_activate)
+    return query.exclude(status__in=[active_value, revoked_value]).filter(
+        invited_at__gte=lowest_datetime)
+
 class MultiuseInvite(models.Model):
     referred_by = models.ForeignKey(UserProfile, on_delete=CASCADE)  # Optional[UserProfile]
     streams: Manager = models.ManyToManyField('Stream')

zerver/tests/test_auth_backends.py

@@ -1415,6 +1415,29 @@ class SocialAuthBase(DesktopFlowTestingLib, ZulipTestCase):
             self.assertEqual(result.status_code, 302)
             self.assertIn('login', result.url)
 
+    @override_settings(TERMS_OF_SERVICE=None)
+    def test_social_auth_invited_as_admin_but_expired(self) -> None:
+        iago = self.example_user("iago")
+        email = self.nonreg_email("alice")
+        name = 'Alice Jones'
+
+        do_invite_users(iago, [email], [], invite_as=PreregistrationUser.INVITE_AS['REALM_ADMIN'])
+        expired_date = timezone_now() - datetime.timedelta(days=settings.INVITATION_LINK_VALIDITY_DAYS + 1)
+        PreregistrationUser.objects.filter(email=email).update(invited_at=expired_date)
+
+        subdomain = 'zulip'
+        realm = get_realm("zulip")
+        account_data_dict = self.get_account_data_dict(email=email, name=name)
+        result = self.social_auth_test(account_data_dict,
+                                       expect_choose_email_screen=True,
+                                       subdomain=subdomain, is_signup=True)
+        self.stage_two_of_registration(result, realm, subdomain, email, name, name,
+                                       self.BACKEND_CLASS.full_name_validated)
+
+        # The invitation is expired, so the user should be created as normal member only.
+        created_user = get_user_by_delivery_email(email, realm)
+        self.assertEqual(created_user.role, UserProfile.ROLE_MEMBER)
+
 class SAMLAuthBackendTest(SocialAuthBase):
     __unittest_skip__ = False
 

zerver/views/auth.py

@@ -50,7 +50,14 @@ from zerver.lib.user_agent import parse_user_agent
 from zerver.lib.users import get_api_key
 from zerver.lib.utils import has_api_key_format
 from zerver.lib.validator import validate_login_email
-from zerver.models import PreregistrationUser, Realm, UserProfile, get_realm, remote_user_to_email
+from zerver.models import (
+    PreregistrationUser,
+    Realm,
+    UserProfile,
+    filter_to_valid_prereg_users,
+    get_realm,
+    remote_user_to_email,
+)
 from zerver.signals import email_on_new_login
 from zproject.backends import (
     AUTH_BACKEND_NAME_MAP,
@@ -159,8 +166,8 @@ def maybe_send_to_registration(request: HttpRequest, email: str, full_name: str=
         # creation or confirm-continue-registration depending on
         # is_signup.
         try:
-            prereg_user = PreregistrationUser.objects.filter(
-                email__iexact=email, realm=realm).latest("invited_at")
+            prereg_user = filter_to_valid_prereg_users(PreregistrationUser.objects.filter(
+                email__iexact=email, realm=realm)).latest("invited_at")
 
             # password_required and full_name data passed here as argument should take precedence
             # over the defaults with which the existing PreregistrationUser that we've just fetched

zerver/worker/queue_processors.py

@@ -89,6 +89,7 @@ from zerver.models import (
     RealmAuditLog,
     UserMessage,
     UserProfile,
+    filter_to_valid_prereg_users,
     flush_per_request_caches,
     get_bot_services,
     get_client,
@@ -324,10 +325,13 @@ class ConfirmationEmailWorker(QueueProcessingWorker):
         if "email" in data:
             # When upgrading from a version up through 1.7.1, there may be
             # existing items in the queue with `email` instead of `prereg_id`.
-            invitee = PreregistrationUser.objects.filter(
-                email__iexact=data["email"].strip()).latest("invited_at")
+            invitee = filter_to_valid_prereg_users(
+                PreregistrationUser.objects.filter(email__iexact=data["email"].strip())
+            ).latest("invited_at")
         else:
-            invitee = PreregistrationUser.objects.filter(id=data["prereg_id"]).first()
+            invitee = filter_to_valid_prereg_users(
+                PreregistrationUser.objects.filter(id=data["prereg_id"])
+            ).first()
             if invitee is None:
                 # The invitation could have been revoked
                 return