Opened 54 minutes ago
#37388 new Uncategorized
Security policy shows insecure example as "correct approach"
| Reported by: | Mike Edmunds | Owned by: | |
|---|---|---|---|
| Component: | Documentation | Version: | 6.1 |
| Severity: | Normal | Keywords: | security |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
The User input must be sanitized section of Django's security policies presents a "not considered valid" vulnerability report using a raw query param as the from_email in a send_mail() call. It then shows the code below as the "correct approach."
But this code still uses user-supplied content as the from_email; it just verifies it looks like an email address first. Sending email from an arbitrary address is at best a poor practice, and often a real vulnerability (in the user's code, not Django). See #37162 where we fixed similar problems in contact form examples throughout the docs.
from django import forms from django.core.mail import send_mail from django.http import JsonResponse class EmailForm(forms.Form): email = forms.EmailField() def my_proof_of_concept(request): form = EmailForm(request.GET) if form.is_valid(): send_mail( "Email subject", "Email body", form.cleaned_data["email"], ["admin@example.com"], ) return JsonResponse(status=200) return JsonResponse(form.errors, status=400)
We could try to rework this code as in the earlier ticket, but I'm not sure sending email is an effective example for this particular security reporting topic. (Django has long prevented email header injection, so the original "not considered valid" code isn't really any more insecure than the "correct approach." It's just more likely to generate an uncaught error on bad input.)
I'm not really sure about a replacement. Maybe something where raw user input is interpolated directly into an html response, rather than being escaped or rendered through a template?