CVE-2023-0440: Observable Discrepancy (CWE-203)
value: Observable Discrepancy in GitHub repository healthchecks/healthchecks prior to v2.6.
CWE description: The product behaves differently or sends different responses under different circumstances in a way that is observable to an unauthorized actor, which exposes security-relevant information about the state of the product, such as whether a particular operation was successful or not.
CVSS v3 base score: 5.3
Published: 2023-01-23T14:15Z
Repository: https://github.com/healthchecks/healthchecks
Language: Python
Files changed: CHANGELOG.md, hc/accounts/forms.py, hc/accounts/tests/test_login.py, hc/accounts/tests/test_signup.py, hc/accounts/views.py, templates/accounts/login_link_sent.html, templates/accounts/signup_result.html
Commit date: 2023-01-23 13:05:49 +0200
Commit message: Fix login and signup views to make email enumeration harder

Diff: diff --git a/CHANGELOG.md b/CHANGELOG.md
index cfe1246f..6f9bcc25 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,10 +15,11 @@ All notable changes to this project will be documented in this file.
 - Fix special character encoding in Signal notifications (#767)
 - Fix project sort order to be case-insensitive everywhere in the UI (#768)
 - Fix special character encoding in project invite emails
 - Fix check transfer between same account's projects when at check limit
 - Fix wording in the invite email when inviting read-only users
+- Fix login and signup views to make email enumeration harder
 
 ## v2.5 - 2022-12-14
 
 ### Improvements
 - Upgrade to fido2 1.1.0 and simplify hc.lib.webauthn
diff --git a/hc/accounts/forms.py b/hc/accounts/forms.py
index a83af82a..3ec40648 100644
--- a/hc/accounts/forms.py
+++ b/hc/accounts/forms.py
@@ -28,15 +28,10 @@ class SignupForm(forms.Form):
     def clean_identity(self):
         v = self.cleaned_data["identity"]
         if len(v) > 254:
             raise forms.ValidationError("Address is too long.")
 
-        if User.objects.filter(email=v).exists():
-            raise forms.ValidationError(
-                "An account with this email address already exists."
-            )
-
         return v
 
     def clean_tz(self):
         # Declare tz as "clean" only if we can find it in hc.lib.tz.all_timezones
         if self.cleaned_data["tz"] in all_timezones:
@@ -58,11 +53,11 @@ class EmailLoginForm(forms.Form):
             raise forms.ValidationError("Too many attempts, please try later.")
 
         try:
             self.user = User.objects.get(email=v)
         except User.DoesNotExist:
-            raise forms.ValidationError("Unknown email address.")
+            self.user = None
 
         return v
 
 
 class PasswordLoginForm(forms.Form):
diff --git a/hc/accounts/tests/test_login.py b/hc/accounts/tests/test_login.py
index ae03f5d8..dad48267 100644
--- a/hc/accounts/tests/test_login.py
+++ b/hc/accounts/tests/test_login.py
@@ -55,10 +55,20 @@ class LoginTestCase(BaseTestCase):
         # The check_token link should have a ?next= query parameter:
         self.assertEqual(len(mail.outbox), 1)
         body = mail.outbox[0].body
         self.assertTrue("/?next=" + self.channels_url in body)
 
+    def test_it_handles_unknown_email(self):
+        form = {"identity": "surprise@example.org"}
+
+        r = self.client.post("/accounts/login/", form)
+        self.assertRedirects(r, "/accounts/login_link_sent/")
+        self.assertIn("auto-login", r.cookies)
+
+        # There should be no sent emails.
+        self.assertEqual(len(mail.outbox), 0)
+
     @override_settings(SECRET_KEY="test-secret")
     def test_it_rate_limits_emails(self):
         # "d60d..." is sha1("alice@example.orgtest-secret")
         obj = TokenBucket(value="em-d60db3b2343e713a4de3e92d4eb417e4f05f06ab")
         obj.tokens = 0
diff --git a/hc/accounts/tests/test_signup.py b/hc/accounts/tests/test_signup.py
index 96d595dd..d19872e4 100644
--- a/hc/accounts/tests/test_signup.py
+++ b/hc/accounts/tests/test_signup.py
@@ -14,11 +14,11 @@ class SignupTestCase(TestCase):
     @override_settings(USE_PAYMENTS=False)
     def test_it_works(self):
         form = {"identity": "alice@example.org", "tz": "Europe/Riga"}
 
         r = self.client.post("/accounts/signup/", form)
-        self.assertContains(r, "Account created")
+        self.assertContains(r, "check your email")
         self.assertIn("auto-login", r.cookies)
 
         # An user should have been created
         user = User.objects.get()
 
@@ -73,17 +73,21 @@ class SignupTestCase(TestCase):
 
         # There should be exactly one user:
         q = User.objects.filter(email="alice@example.org")
         self.assertTrue(q.exists)
 
-    def test_it_checks_for_existing_users(self):
+    def test_it_handles_existing_users(self):
         alice = User(username="alice", email="alice@example.org")
         alice.save()
 
         form = {"identity": "alice@example.org", "tz": ""}
         r = self.client.post("/accounts/signup/", form)
-        self.assertContains(r, "already exists")
+        self.assertContains(r, "check your email")
+        self.assertIn("auto-login", r.cookies)
+
+        # It should not send an email
+        self.assertEqual(len(mail.outbox), 0)
 
     def test_it_checks_syntax(self):
         form = {"identity": "alice at example org", "tz": ""}
         r = self.client.post("/accounts/signup/", form)
         self.assertContains(r, "Enter a valid email address")
@@ -99,10 +103,10 @@ class SignupTestCase(TestCase):
     @override_settings(USE_PAYMENTS=False)
     def test_it_ignores_bad_tz(self):
         form = {"identity": "alice@example.org", "tz": "Foo/Bar"}
 
         r = self.client.post("/accounts/signup/", form)
-        self.assertContains(r, "Account created")
+        self.assertContains(r, "check your email")
         self.assertIn("auto-login", r.cookies)
 
         profile = Profile.objects.get()
         self.assertEqual(profile.tz, "UTC")
diff --git a/hc/accounts/views.py b/hc/accounts/views.py
index 9759d765..f5b9c2f9 100644
--- a/hc/accounts/views.py
+++ b/hc/accounts/views.py
@@ -159,14 +159,15 @@ def login(request):
             if magic_form.is_valid():
                 redirect_url = request.GET.get("next")
                 if not _allow_redirect(redirect_url):
                     redirect_url = None
 
-                profile = Profile.objects.for_user(magic_form.user)
-                profile.send_instant_login_link(redirect_url=redirect_url)
-                response = redirect("hc-login-link-sent")
+                if magic_form.user:
+                    profile = Profile.objects.for_user(magic_form.user)
+                    profile.send_instant_login_link(redirect_url=redirect_url)
 
+                response = redirect("hc-login-link-sent")
                 # check_token looks for this cookie to decide if
                 # it needs to do the extra POST step.
                 response.set_cookie("auto-login", "1", max_age=300, httponly=True)
                 return response
 
@@ -199,20 +200,20 @@ def signup(request):
 
     ctx = {}
     form = forms.SignupForm(request.POST)
     if form.is_valid():
         email = form.cleaned_data["identity"]
-        tz = form.cleaned_data["tz"]
-        user = _make_user(email, tz)
-        profile = Profile.objects.for_user(user)
-        profile.send_instant_login_link()
-        ctx["created"] = True
+        if not User.objects.filter(email=email).exists():
+            tz = form.cleaned_data["tz"]
+            user = _make_user(email, tz)
+            profile = Profile.objects.for_user(user)
+            profile.send_instant_login_link()
     else:
         ctx = {"form": form}
 
     response = render(request, "accounts/signup_result.html", ctx)
-    if ctx.get("created"):
+    if "form" not in ctx:
         response.set_cookie("auto-login", "1", max_age=300, httponly=True)
 
     return response
 
 
diff --git a/templates/accounts/login_link_sent.html b/templates/accounts/login_link_sent.html
index b5aebc2c..3aec22e9 100644
--- a/templates/accounts/login_link_sent.html
+++ b/templates/accounts/login_link_sent.html
@@ -1,18 +1,18 @@
 {% extends "base.html" %}
+{% load hc_extras %}
 
 {% block content %}
 <div class="row">
     <div class="col-sm-6 col-sm-offset-3">
         <div class="hc-dialog">
-            <h1>Login Link Sent!</h1>
+            <h1>Check Your Email!</h1>
             <br />
             <p>
-                We've sent you an email with login instructions.
-                Please check your inbox!
+                If a {% site_name %} account exists for this email address,
+                you will receive a login link in your email shortly.
             </p>
-
         </div>
     </div>
 </div>
 
 {% endblock %}
diff --git a/templates/accounts/signup_result.html b/templates/accounts/signup_result.html
index 5a096fda..1c049425 100644
--- a/templates/accounts/signup_result.html
+++ b/templates/accounts/signup_result.html
@@ -1,7 +1,5 @@
 {% for error in form.identity.errors %}
 <p class="text-danger">{{ error }}</p>
-{% endfor %}
-
-{% if created %}
-<p class="text-success">Account created, please check your email!</p>
-{% endif %}
\ No newline at end of file
+{% empty %}
+<p class="text-success">Please check your email!</p>
+{% endfor %}
\ No newline at end of file

Vulnerable code:         if User.objects.filter(email=v).exists():
            raise forms.ValidationError(
                "An account with this email address already exists."
            )

            raise forms.ValidationError("Unknown email address.")
        self.assertContains(r, "Account created")
    def test_it_checks_for_existing_users(self):
        self.assertContains(r, "already exists")
        self.assertContains(r, "Account created")
                profile = Profile.objects.for_user(magic_form.user)
                profile.send_instant_login_link(redirect_url=redirect_url)
                response = redirect("hc-login-link-sent")
        tz = form.cleaned_data["tz"]
        user = _make_user(email, tz)
        profile = Profile.objects.for_user(user)
        profile.send_instant_login_link()
        ctx["created"] = True
    if ctx.get("created"):
            <h1>Login Link Sent!</h1>
                We've sent you an email with login instructions.
                Please check your inbox!

{% endfor %}

{% if created %}
<p class="text-success">Account created, please check your email!</p>
{% endif %}
Fixed code: - Fix login and signup views to make email enumeration harder
            self.user = None
    def test_it_handles_unknown_email(self):
        form = {"identity": "surprise@example.org"}

        r = self.client.post("/accounts/login/", form)
        self.assertRedirects(r, "/accounts/login_link_sent/")
        self.assertIn("auto-login", r.cookies)

        # There should be no sent emails.
        self.assertEqual(len(mail.outbox), 0)

        self.assertContains(r, "check your email")
    def test_it_handles_existing_users(self):
        self.assertContains(r, "check your email")
        self.assertIn("auto-login", r.cookies)

        # It should not send an email
        self.assertEqual(len(mail.outbox), 0)
        self.assertContains(r, "check your email")
                if magic_form.user:
                    profile = Profile.objects.for_user(magic_form.user)
                    profile.send_instant_login_link(redirect_url=redirect_url)
                response = redirect("hc-login-link-sent")
        if not User.objects.filter(email=email).exists():
            tz = form.cleaned_data["tz"]
            user = _make_user(email, tz)
            profile = Profile.objects.for_user(user)
            profile.send_instant_login_link()
    if "form" not in ctx:
{% load hc_extras %}
            <h1>Check Your Email!</h1>
                If a {% site_name %} account exists for this email address,
                you will receive a login link in your email shortly.
{% empty %}
<p class="text-success">Please check your email!</p>
{% endfor %}