Skip to content

Commit

Permalink
Merge pull request #4491 from freelawproject/4466-fix-check-email-in-…
Browse files Browse the repository at this point in the history
…register-success-view

fix(users): Prevent text Injection in Email Query Parameter
  • Loading branch information
mlissner authored Sep 24, 2024
2 parents 5f168d2 + 7c84560 commit 0318c31
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 0 deletions.
39 changes: 39 additions & 0 deletions cl/users/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,45 @@ async def test_redirects(self) -> None:
% next_param,
)

async def test_prevent_text_injection_in_success_registration(self):
"""Can we handle text injection attacks?"""
evil_text = "visit https://evil.com/malware.exe to win $100 giftcard"
url_params = [
# A safe redirect and email
(reverse("faq"), "test@free.law", False),
# Text injection attack
(reverse("faq"), evil_text, True),
# open redirect and text injection attack
("https://evil.com&email=e%40e.net", evil_text, True),
]

for next_param, email, is_evil in url_params:
url = "{host}{path}?next={next}&email={email}".format(
host=self.live_server_url,
path=reverse("register_success"),
next=next_param,
email=email,
)
response = await self.async_client.get(url)
with self.subTest("Checking url", url=url):
if is_evil:
self.assertNotIn(
email,
response.content.decode(),
msg="'%s' found in HTML of response. This indicates a "
"potential security vulnerability. The view likely "
"failed to properly validate it." % email,
)
else:
self.assertIn(
email,
response.content.decode(),
msg="'%s' not found in HTML of response. This suggests a "
"a potential issue with the validation logic. The email "
"address may have been incorrectly identified as invalid"
% email,
)

async def test_login_redirects(self) -> None:
"""Do we allow good redirects in login while banning bad ones?"""
next_params = [
Expand Down
8 changes: 8 additions & 0 deletions cl/users/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
from django.contrib.auth.decorators import login_required
from django.contrib.auth.models import User
from django.contrib.auth.views import PasswordResetView
from django.core.exceptions import SuspiciousOperation, ValidationError
from django.core.mail import send_mail
from django.core.paginator import EmptyPage, PageNotAnInteger, Paginator
from django.core.validators import validate_email
from django.db.models import Count, F
from django.http import (
HttpRequest,
Expand Down Expand Up @@ -547,6 +549,12 @@ def register_success(request: HttpRequest) -> HttpResponse:
they left off."""
redirect_to = get_redirect_or_abort(request, "next")
email = request.GET.get("email", "")
if email:
try:
validate_email(email)
except ValidationError:
raise SuspiciousOperation("Invalid Email address")

default_from = parseaddr(settings.DEFAULT_FROM_EMAIL)[1]
return TemplateResponse(
request,
Expand Down

0 comments on commit 0318c31

Please sign in to comment.