[Django] #30530: url `path` accepts newlines in various places.
Django
----------------------------------------------+------------------------
Reporter: Sjoerd Job Postmus | Owner: nobody
Type: Bug | Status: new
Component: Core (URLs) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
----------------------------------------------+------------------------
Consider the following simplified `urls.py`.
{{{
from django.http import HttpResponse
from django.urls import path
def path_view(request):
return HttpResponse('<pre>===>' + request.path + '<===</pre>')
def render_something(request, something):
return HttpResponse('<pre>===>' + something + '<===</pre>')
urlpatterns = [
path('hello/', path_view),
path('foo/<something>/bar/', render_something),
]
}}}
By accessing `http://localhost:8000/hello/%0a`, it's clear that the
newline is accepted in the URL. This is because the underlying logic uses
a `$` in the regular expression, instead of `\Z`..
By accessing `http://localhost:8000/foo/hello%0aworld/bar/`, it's clear
that the default `str` converter accepts anywhere in the segment. This is
because it uses a negative match `[^/]+`, which happily accepts a newline
character (both `%0a` and `%0d`).
I propose changing the `$` to `\Z`, and the negative match to `[^/\r\n]+`.
I would also suggest changing the documentation on the `re_path` to
suggest `\Z` instead of `$`, though that may be more controversial.
--
Ticket URL: <https://code.djangoproject.com/ticket/30530>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Type: Bug | Status: new
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by felixxm):
Thanks for this report, however is there any reason to add this
restriction? I don't see any issue in accepting encoded newline characters
in URL parameters. Moreover this behavior is
[https://docs.djangoproject.com/en/2.2/topics/http/urls/#path-converters
documented] and can be used by users.
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:1>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => wontfix
Comment:
Escaped newlines are legitimate in URLs (and required in cases, e.g.
#24962).
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:2>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Sjoerd Job Postmus):
In that case, isn't the `path` converter incorrect in **not** accepting
newlines?
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:3>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
The converters are as they are as a result of design decisions made when
introducing the feature. Anyone needing different can implement custom
converters.
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:4>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* resolution: wontfix => fixed
Comment:
In [changeset:"22bd17488159601bf0741b70ae7932bffea8eced" 22bd174]:
{{{
#!CommitTicketReference repository=""
revision="22bd17488159601bf0741b70ae7932bffea8eced"
[3.1.x] Fixed #30530, CVE-2021-44420 -- Fixed potential bypass of an
upstream access control based on URL paths.
Thanks Sjoerd Job Postmus and TengMA(@te3t123) for reports.
Backport of d4dcd5b9dd9e462fec8220e33e3e6c822b7e88a6 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:5>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: dev
Severity: Normal | Resolution: fixed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"d4dcd5b9dd9e462fec8220e33e3e6c822b7e88a6" d4dcd5b9]:
{{{
#!CommitTicketReference repository=""
revision="d4dcd5b9dd9e462fec8220e33e3e6c822b7e88a6"
Fixed #30530, CVE-2021-44420 -- Fixed potential bypass of an upstream
access control based on URL paths.
Thanks Sjoerd Job Postmus and TengMA(@te3t123) for reports.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:7>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: dev
Severity: Normal | Resolution: fixed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"20b9ad36ff0558b819659a10a9734262367750be" 20b9ad36]:
{{{
#!CommitTicketReference repository=""
revision="20b9ad36ff0558b819659a10a9734262367750be"
[4.0.x] Fixed #30530, CVE-2021-44420 -- Fixed potential bypass of an
upstream access control based on URL paths.
Thanks Sjoerd Job Postmus and TengMA(@te3t123) for reports.
Backport of d4dcd5b9dd9e462fec8220e33e3e6c822b7e88a6 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:6>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: dev
Severity: Normal | Resolution: fixed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"333c65603032c377e682cdbd7388657a5463a05a" 333c6560]:
{{{
#!CommitTicketReference repository=""
revision="333c65603032c377e682cdbd7388657a5463a05a"
[3.2.x] Fixed #30530, CVE-2021-44420 -- Fixed potential bypass of an
upstream access control based on URL paths.
Thanks Sjoerd Job Postmus and TengMA(@te3t123) for reports.
Backport of d4dcd5b9dd9e462fec8220e33e3e6c822b7e88a6 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:8>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: dev
Severity: Normal | Resolution: fixed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"7cf7d74e8a754446eeb85cacf2fef1247e0cb6d7" 7cf7d74]:
{{{
#!CommitTicketReference repository=""
revision="7cf7d74e8a754446eeb85cacf2fef1247e0cb6d7"
[2.2.x] Fixed #30530, CVE-2021-44420 -- Fixed potential bypass of an
upstream access control based on URL paths.
Thanks Sjoerd Job Postmus and TengMA(@te3t123) for reports.
Backport of d4dcd5b9dd9e462fec8220e33e3e6c822b7e88a6 from main.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:9>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: dev
Severity: Normal | Resolution: fixed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:10>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by GitHub <noreply@…>):
In [changeset:"5de12a369a7b2231e668e0460c551c504718dbf6" 5de12a36]:
{{{
#!CommitTicketReference repository=""
revision="5de12a369a7b2231e668e0460c551c504718dbf6"
Refs #33365, Refs #30530 -- Doc'd re_path() behavior change in Django
2.2.25, 3.1.14, and 3.2.10.
Follow up to d4dcd5b9dd9e462fec8220e33e3e6c822b7e88a6.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:11>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"267a743bf253a4e0703c0257a5df7774116c3194" 267a743b]:
{{{
#!CommitTicketReference repository=""
revision="267a743bf253a4e0703c0257a5df7774116c3194"
[4.0.x] Refs #33365, Refs #30530 -- Doc'd re_path() behavior change in
Django 2.2.25, 3.1.14, and 3.2.10.
Follow up to d4dcd5b9dd9e462fec8220e33e3e6c822b7e88a6.
Backport of 5de12a369a7b2231e668e0460c551c504718dbf6 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:12>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"ae242235db910a94ea476b7a3efacf8dbab31c4f" ae242235]:
{{{
#!CommitTicketReference repository=""
revision="ae242235db910a94ea476b7a3efacf8dbab31c4f"
[3.2.x] Refs #33365, Refs #30530 -- Doc'd re_path() behavior change in
Django 2.2.25, 3.1.14, and 3.2.10.
Follow up to d4dcd5b9dd9e462fec8220e33e3e6c822b7e88a6.
Backport of 5de12a369a7b2231e668e0460c551c504718dbf6 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:13>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"0b8a0296bfd30748f08021834e95cdae241686e8" 0b8a0296]:
{{{
#!CommitTicketReference repository=""
revision="0b8a0296bfd30748f08021834e95cdae241686e8"
[3.1.x] Refs #33365, Refs #30530 -- Doc'd re_path() behavior change in
Django 2.2.25, 3.1.14, and 3.2.10.
Follow up to d4dcd5b9dd9e462fec8220e33e3e6c822b7e88a6.
Backport of 5de12a369a7b2231e668e0460c551c504718dbf6 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:14>
Django
Reporter: Sjoerd Job Postmus | Owner: nobody
Component: Core (URLs) | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"b87820668e7bd519dbc05f6ee46f551858fb1d6d" b8782066]:
{{{
#!CommitTicketReference repository=""
revision="b87820668e7bd519dbc05f6ee46f551858fb1d6d"
[2.2.x] Refs #33365, Refs #30530 -- Doc'd re_path() behavior change in
Django 2.2.25, 3.1.14, and 3.2.10.
Follow up to d4dcd5b9dd9e462fec8220e33e3e6c822b7e88a6.
Backport of 5de12a369a7b2231e668e0460c551c504718dbf6 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30530#comment:15>