From b34a23feaa4962f5f33f155d86eb4afd4e3a4687 Mon Sep 17 00:00:00 2001 From: Eduardo Rosendo Date: Mon, 18 Nov 2024 13:25:59 -0400 Subject: [PATCH 1/2] fix(users): Enforce validation on first_name for increase security --- cl/users/forms.py | 12 ++++++++++++ cl/users/tests.py | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/cl/users/forms.py b/cl/users/forms.py index 4ec758d611..ab151b8ea7 100644 --- a/cl/users/forms.py +++ b/cl/users/forms.py @@ -1,3 +1,5 @@ +import re + from disposable_email_domains import blocklist from django import forms from django.contrib.auth import authenticate @@ -188,6 +190,16 @@ def clean_email(self): ) return email + def clean_first_name(self): + first_name = self.cleaned_data.get("first_name") + if not re.match( + r"""[^!"#$%&()*+,./:;<=>?@[\]_{|}~]+$""", first_name, re.IGNORECASE + ): + raise forms.ValidationError( + "First name must not contain any special characters." + ) + return first_name + class EmailConfirmationForm(forms.Form): email = forms.EmailField( diff --git a/cl/users/tests.py b/cl/users/tests.py index 26d997d139..c03318014e 100644 --- a/cl/users/tests.py +++ b/cl/users/tests.py @@ -275,6 +275,50 @@ async def test_signing_in(self) -> None: ) self.assertRedirects(r, "/") + async def test_registration_rejects_malicious_first_name_input( + self, + ) -> None: + tests = ( + # Invalid + ("evil.com", False), + ("http://test", False), + ("email@test.test", False), + ("/test/", False), + # Valid + ("My fullname", True), + ("Test Test", True), + ("Éric Terrien-Pascal", True), + ("Tel'c", True), + ) + for first_name, is_valid in tests: + with self.subTest( + f"Trying to register using {first_name} as first name.", + first_name=first_name, + is_valid=is_valid, + ): + r = await self.async_client.post( + reverse("register"), + { + "username": "aamon", + "email": "user@free.law", + "password1": "a", + "password2": "a", + "first_name": first_name, + "last_name": "Marquis of Hell", + "skip_me_if_alive": "", + }, + ) + if not is_valid: + self.assertIn( + "First name must not contain any special characters.", + r.content.decode(), + ) + else: + self.assertNotIn( + "First name must not contain any special characters.", + r.content.decode(), + ) + async def test_confirming_an_email_address(self) -> None: """Tests whether we can confirm the case where an email is associated with a single account. From 1fd6662abbcbb344b5b03ac1fac9bffcf12ea84a Mon Sep 17 00:00:00 2001 From: Eduardo Rosendo Date: Mon, 18 Nov 2024 21:51:48 -0400 Subject: [PATCH 2/2] refactor(users): Refine logic to validate first_name field --- cl/users/forms.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cl/users/forms.py b/cl/users/forms.py index ab151b8ea7..022b08a632 100644 --- a/cl/users/forms.py +++ b/cl/users/forms.py @@ -192,9 +192,7 @@ def clean_email(self): def clean_first_name(self): first_name = self.cleaned_data.get("first_name") - if not re.match( - r"""[^!"#$%&()*+,./:;<=>?@[\]_{|}~]+$""", first_name, re.IGNORECASE - ): + if re.search(r"""[!"#$%&()*+,./:;<=>?@[\]_{|}~]+""", first_name): raise forms.ValidationError( "First name must not contain any special characters." )