Opened 4 years ago
Closed 4 years ago
#32571 closed Bug (fixed)
CsrfViewMiddleware assumes referer header can be parsed
| Reported by: | Chris Jerdonek | Owned by: | AdamDonna |
|---|---|---|---|
| Component: | CSRF | Version: | 3.1 |
| Severity: | Normal | Keywords: | referer, CSRF, CsrfViewMiddleware |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Django's CsrfViewMiddleware assumes that the HTTP referer header is valid when checking it. Specifically, it doesn't handle the case of urlparse() raising a ValueError in this line (e.g. for urls like 'https://['):
https://github.com/django/django/blob/45814af6197cfd8f4dc72ee43b90ecde305a1d5a/django/middleware/csrf.py#L244
Change History (6)
comment:1 by , 4 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
follow-up: 3 comment:2 by , 4 years ago
comment:3 by , 4 years ago
Replying to AdamDonna:
Should the response in this scenario be something like this line? Or would a different response reason make more sense https://github.com/django/django/blob/45814af6197cfd8f4dc72ee43b90ecde305a1d5a/django/middleware/csrf.py#L248
Yes, we should reject immediately.
follow-up: 5 comment:4 by , 4 years ago
Great i've got a PR up for this. Are there any docs that need to be updated?
https://github.com/django/django/pull/14151
comment:5 by , 4 years ago
| Has patch: | set |
|---|---|
| Owner: | changed from to |
| Status: | new → assigned |
| Triage Stage: | Accepted → Ready for checkin |
Replying to AdamDonna:
Great i've got a PR up for this. Are there any docs that need to be updated?
https://github.com/django/django/pull/14151
No need, thanks.
Should the response in this scenario be something like this line? Or would a different response reason make more sense https://github.com/django/django/blob/45814af6197cfd8f4dc72ee43b90ecde305a1d5a/django/middleware/csrf.py#L248