[Django] #32571: CsrfViewMiddleware assumes referer header can be parsed
Django
-------------------------------------+-------------------------------------
Reporter: Chris | Owner: nobody
Jerdonek |
Type: Bug | Status: new
Component: CSRF | Version: 3.1
Severity: Normal | Keywords:
Triage Stage: | referer,CSRF,CsrfViewMiddleware
Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
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
--
Ticket URL: <https://code.djangoproject.com/ticket/32571>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: CSRF | Version: 3.1
Keywords: | Triage Stage: Accepted
referer,CSRF,CsrfViewMiddleware |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/32571#comment:1>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: CSRF | Version: 3.1
Keywords: | Triage Stage: Accepted
referer,CSRF,CsrfViewMiddleware |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by 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
--
Ticket URL: <https://code.djangoproject.com/ticket/32571#comment:2>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: CSRF | Version: 3.1
Keywords: | Triage Stage: Accepted
referer,CSRF,CsrfViewMiddleware |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:2 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32571#comment:3>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: CSRF | Version: 3.1
Keywords: | Triage Stage: Accepted
referer,CSRF,CsrfViewMiddleware |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by 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
--
Ticket URL: <https://code.djangoproject.com/ticket/32571#comment:4>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: assigned
Component: CSRF | Version: 3.1
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
referer,CSRF,CsrfViewMiddleware | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => AdamDonna
* status: new => assigned
* has_patch: 0 => 1
* stage: Accepted => Ready for checkin
Comment:
Replying to [comment:4 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32571#comment:5>
Django
-------------------------------------+-------------------------------------
Component: CSRF | Version: 3.1
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
referer,CSRF,CsrfViewMiddleware | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"e49fdfa405fcacb59d7ff2f321a7ddbc65dfc68b" e49fdfa4]:
{{{
#!CommitTicketReference repository=""
revision="e49fdfa405fcacb59d7ff2f321a7ddbc65dfc68b"
Fixed #32571 -- Made CsrfViewMiddleware handle invalid URLs in Referer
header.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32571#comment:6>