[Django] #21231: Limiting the number of variables and files that a POST request can contain
Django
-------------------------------+--------------------
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
== The Problem ==
When deploying a django application one should be able to asses what
will be *the absolute maximum* of memory consumed by each serving
process/thread. Failing to make this assessment correctly may leave the
application open to denial-of-service attacks. For example, when
deploying with apache, an excessive number of threads should be
available in order protect yourself from slowloris-type attacks. Also,
in order to limit the maximum memory and CPU usage under attack
"LimitRequestBody" should be set to a reasonably small value (1MB for
example). So, here are the naive math: //"100 parallel requests"// X
//"1MB
per request"// = //"100MB maximum memory consumed"//
The problem is that the naive math is wrong. A single 1MB POST request
may require *much* more that 1MB to process. 1MB of
"application/x-www-form-urlencoded" type may contain 200,000 short
name-value pairs, which when fetched into the interpreter consume tens of
megabytes (depending on the platform, probably 10-50MB). So the
correct math would be: //"100 parallel requests"// X //"50MB per
request"// =
//"5000MB maximum memory consumed"//. Probably too much for many systems.
As far as I know, django does not offer an easy way to protect against
such edge cases (lots of simultaneous malicious POST request that consume
unexpected amounts of memory).
== The proposal ==
Introduce two new settings, limiting the maximum number of entities in
a POST request:
**FORM_DATA_URLENCODED_MAX_VARS**: the maximum number of name-value
pairs in the case of "application/x-www-form-urlencoded". (no limit by
default)
**FORM_DATA_MULTIPART_MAX_PARTS**: the maximum number of parts in the
case of "multipart/form-data". (no limit by default)
In case those limits are exceeded, the request must be discarded
without being fetched/parsed into memory. (In our example, it would
consume only 1MB instead of 50.)
--
Ticket URL: <https://code.djangoproject.com/ticket/21231>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: closed
Component: HTTP handling | 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 aaugustin):
* status: new => closed
* needs_better_patch: => 0
* resolution: => wontfix
* needs_tests: => 0
* needs_docs: => 0
Comment:
> one should be able to asses what will be *the absolute maximum* of
memory consumed by each serving process/thread.
I'm sorry, but I fail to see how this could be achieved in Python. Memory
use will grow until the GC kicks in, and that isn't under your control.
> A single 1MB POST request may require *much* more that 1MB to process
POST data is a rather small part of the memory footprint of a standard
Django application. It isn't unusal to allocate 100MB or 200MB of RAM per
process in a Django application server.
Your proposal doesn't appear to be predicated on a very good understanding
of memory management in Python and memory use in Django applications; I'm
not convinced it has enough value to warrant introducing two settings.
Besides, Django doesn't provide a production-grade application server, and
doesn't plan to (see the docs for runserver). The common way to avoid
excessive memory usage is to have the application server manage it,
generally by killing processes that exceed a given threshold.
If you want to discuss possible ways to restrict memory use in Django, I
suggest starting a discussion on the django-developers mailing-list. Thank
you.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:1>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by epandurski@…):
* status: closed => new
* resolution: wontfix =>
Comment:
Obviously, I have not been sufficiently clear in explaining the problem.
I agree that as you say "It isn't unusual to allocate 100MB or 200MB of
RAM per process in a Django application server", but it is not usual to
allocate 50MB of RAM *for processing a single request*. For example, you
may have 1 process (200MB) and 100 threads (say additional 100MB
combined), and that's OK, but when you get a blast of requests, continuing
malicious POST-data, each and every thread will have to create a huge
django request-object (100 X 50 = 5000MB combined).
Memory is a complex problem, and it might very well be the case that I did
not get it right. The last reply, though, doesn't appear to be predicated
on a very thorough understanding of the proposal.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:2>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
I guess so.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:3>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timo):
I'm also not convinced this is something that Django needs to provide.
Your best bet to get this accepted is to start a mailing list thread as
suggested above.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:4>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by epandurski@…):
Well, I do not see how a good a and configurable validation of the HTTP
input is not something that Django needs to provide. The proposal also
will help with the PYTHONHASHSEED-problem (maliciously crafted POST that
results in O(n*n) dict-behaviour).
In any case I will not spend more energy to persuade anyone to the merits
of my proposal (Honestly I do not care that much about Django). I will
rather patch django when I deploy a critical web-app.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:5>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by mjtamlyn):
If you have a patch which you believe works and solves this problem, then
we will gladly consider it for inclusion upstream. That of course assumes
that we can demonstrate that the changes work correctly and don't
introduce any other significant issues. If these do provide a suitable
security benefit against DDOS style attacks as then it may be worth
turning this on by default.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:6>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by epandurski@…):
I do not have a patch, and probably will not have soon, if ever. I
discovered this problem while I was analysing the possible weaknesses of a
web-app of mine that *may in the future* be installed as a business
critical web-app (http://sourceforge.net/projects/cmb/).
So I decided to report my findings.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:7>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: closed
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by mjtamlyn):
* status: new => closed
* resolution: => needsinfo
Comment:
Ok, I'm going to close this for now, largely as I agree with Aymeric that
this will likely be exceedingly difficult (if possible) to implement in
python, and I personally don't see it as a major weakness as it can be
managed fairly easily by nginx or a similar web server.
If you do at some point write a patch which manages this directly in
Django, please feel free to reopen this issue with a concrete patch and
we'll see what happens.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:8>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by epandurski@…):
* status: closed => new
* resolution: needsinfo =>
Comment:
I think I have a nicely working patch for this issue. Here it is:
https://github.com/epandurski/django/commit/9ae456e4280843e87a24e508131dfd9919b20db7
What do you think?
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:9>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akaariai):
I have a feeling this doesn't belong into Django. My hunch is that this
will be solved only partially, and there are ways to circumvent the
protections Django has. In addition, it is likely that Apache or nginx
have already solved this problem for us.
Not closing this as I don't understand HTTP protocol that well...
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:10>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by epandurski@…):
It is a hot potato, isn't it?
I think this problem can be solved using an apache module, but I as far as
I know, none of the standard modules does the job. (I assume mod_security
could be helpful, but I do not know much about mod_security).
I think the problem is much easier to solve in Python/Django than as as a
web-server module, exactly because it is not trivial and needs to allow
flexible settings. I agree that my patch is quite incomplete, and I can
think of other things that should be done as well. But those things are
not very hard to do in python (as you can see, the my patch is trivial).
Another important factor is that web developer as a whole do not
understand those problems, and do not even know that they exists.
Unfortunately, this does not make problems to disappear. Django could lead
the way and promote good security practices.
Think about this simple example scenario: I have to allow users to upload
big files (say 20MB) in my Django app, so I set my "LimitRequestBody" to
20MB. Then suddenly all forms in my Django application should be able to
"swallow" 20MB of url-endoded data without eating up my CPU and memory!
Also, how many uploaded files a 20MB POST request can contain? 200,000
should be a good guess. So, how much memory and CPU will consume the
execution of 200,000 upload-handlers?
I think Django core developers should make a design decision about how to
solve those problems, and what the web developers should be advised to do
about them.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:11>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by epandurski@…):
I did some additional thinking about what settings should be available to
the user so that he/she can have sufficient control over the validation of
the POST-ed raw data. I believe that exactly 3 parameters are needed:
**FORM_DATA_MAX_FILES_COUNT** -- the maximum number of file-parts in the
case of "multipart/form-data". (no limit by default)
**FORM_DATA_MAX_FIELDS_COUNT** -- the maximum number of name-value pairs
in the case of "application/x-www-form-urlencoded", and the maximum number
of field-parts in case of "multipart/form-data". (no limit by default)
**FORM_DATA_MAX_FIELDS_TOTAL_LENGTH** -- the maximum total length of the
posted fields (both for "application/x-www-form-urlencoded" and "multipart
/form-data"). This is the sum of the lengths of all field-names and field-
values). Reading from the input socket should not continue after this
length-limit has been exceeded. (no limit by default)
Implementing those should be relatively easy (not much more complex than
my proposed patch).
//More suitable names for these settings may be decided.//
One more think, that I forgot to mention: I believe Django is the proper
place to validate this things, because they are part of the HTTP request
body (the payload). HTTP does not restrict the format of the request
payload (as far as I know), and generally it depends of the MIME-type
stated in the HTTP-header (application/x-www-form-urlencoded, multipart
/form-data, etc.) You should not expect Apache to make such a deep-
inspection of request's payload.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:12>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by zerok):
I agree that these settings would be nice, but I agree with Aymeric et al.
that Django is perhaps not the best place for it. Since we are talking
about a memory sizing issue it would be preferable to stop processing such
requests as early as possible. Since Django is usually situated at or
close to the end of the request processing chain, something before it
handling this issue would be preferable. I totally understand that Apache
usually is not expected to the deep-request-inspection but at the least
the application server should be able to do that. Going down to the
application framework or the application itself for something like that
should be only a last resort.
As reference from other environments, Tomcat has a maxParameterCount [#p1
(1)]. Gunicorn offers something similar but sadly only for headers [#p2
(2)]. Regarding the number of file-parts, this is something mod_security
already offers [#p3 (3)].
[=#p1 (1)] http://tomcat.apache.org/tomcat-7.0-doc/config/http.html
[=#p2 (2)] http://docs.gunicorn.org/en/latest/configure.html#limit-
request-fields
[=#p3 (3)] https://github.com/SpiderLabs/ModSecurity/wiki/Reference-Manual
#wiki-SecUploadFileLimit
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:13>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by epandurski@…):
"I totally understand that Apache usually is not expected to the deep-
request-inspection but at the least the application server should be able
to do that."
The application server may serve lots of web applications, each one
needing a different validation policy. Only the web application knows what
it does with the input data, and therefore only the application (or the
application framework) is in a position to correctly validate the input.
I agree that it is good idea to do input validation as early as possible,
and be as isolated as possible from the rest. I do not see, however, where
else this functionality can be inserted in the Django framework. I do not
think it can be implemented as Django middleware. Suggestions?
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:14>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by tomchristie):
I agree that this is something we should be considering. Take the
following...
{{{
import time
from django.http import QueryDict
content = ('a=1&' * 2500000).rstrip('&')
d = QueryDict(content, encoding='utf-8');
}}}
That represents parsing a single valid 10MB URLEncoded POST request. On
my machine it consumes ~350MB and 100% CPU for ~30secs.
There's no built-in configurable protection in either Apache or Nginx
against that type of request without having also installed optional
plugins, and I'd consider `max_client_body_size = 10m` to be a perfectly
reasonable nginx setting.
I'll defer to the core dev team on this, but request parsing really does
seem to me to be the domain of the application to deal with, rather than
the server. For example, suppose you wanted the application to deal with
JSON requests that can contain base64 encoded file fields, or some other
content type that can contain a mix of file uploads and standard data -
the server has no way of parsing and inspecting if the requests are
malicious or not. As a further example - would we expect the server to be
responsible for parsing and validating against malicious XML payloads and
other content types that the application might parse?
FWIW, I believe that PHP includes
[http://stackoverflow.com/questions/8710185/new-limit-within-php-1000
-fields-per-post-does-someone-know-if-the-number-can built-in protection]
against this type of request, I've not managed to find information about
other languages/frameworks.
The simplest option would prob. be similar to the
`FORM_DATA_MAX_FIELDS_TOTAL_LENGTH` already noted, but perhaps tweak the
semantics slightly so that it's simply a setting specifying the maximum
allowable request size *excluding* any file upload parts
`REQUEST_MAX_DATA_SIZE` or similar. That'd allow the developer to
differentiate between the maximum allowable file upload size per-request,
and the maximum allowable in-memory data structures per-request.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:15>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:16>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
OK — I'm convinced.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:17>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by edevil):
* cc: andre.cruz@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:18>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by apollo13):
* cc: apollo13 (added)
Comment:
Replying to [comment:12 epandurski@…]:
> **FORM_DATA_MAX_FIELDS_TOTAL_LENGTH** -- the maximum total length of the
posted fields (both for "application/x-www-form-urlencoded" and "multipart
/form-data"). This is the sum of the lengths of all field-names and field-
values). Reading from the input socket should not continue after this
length-limit has been exceeded. (no limit by default)
Isn't that covered by Apache's LimitRequestBody already?
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:19>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by epandurski@…):
> Isn't that covered by Apache's LimitRequestBody already?
I almost have forgotten what were my intentions with the
"FORM_DATA_MAX_FIELDS_TOTAL_LENGTH" field :)
I believe they are almost the same, except for 2 things:
1) LimitRequestBody's context is "server config".
"FORM_DATA_MAX_FIELDS_TOTAL_LENGTH" is specific to the web-application.
There might be many different applications running on one server, doing
different CPU/memory intensive stuff.
2) FORM_DATA_MAX_FIELDS_TOTAL_LENGTH is only about "application/x-www-
form-urlencoded" and "multipart/form-data" MIME types, which I believe are
the only MIME types which content django may decide to load into memory
(is that correct?). Notice that somebody may want to allow POST-ing other
MIME-types of big data, and yet have his/her django web-application
protected from DoS attacks.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:20>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by apollo13):
Replying to [comment:20 epandurski@…]:
> 1) LimitRequestBody's context is "server config".
"FORM_DATA_MAX_FIELDS_TOTAL_LENGTH" is specific to the web-application.
There might be many different applications running on one server, doing
different CPU/memory intensive stuff.
`LimitRequestBody` can be set per file/location/vhost etc, so that
wouldn't count
> 2) FORM_DATA_MAX_FIELDS_TOTAL_LENGTH is only about "application/x-www-
form-urlencoded" and "multipart/form-data" MIME types, which I believe are
the only MIME types which content django may decide to load into memory
(is that correct?). Notice that somebody may want to allow POST-ing other
MIME-types of big data, and yet have his/her django web-application
protected from DoS attacks.
I might be wrong here: But if my app allows file uploads and I set this
value to be equivalent to 1GB (I have big files ;)) I could still easily
DOS the server; in my opinion it would make more sense if that would limit
post data excluding file data.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:21>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by epandurski@…):
> LimitRequestBody can be set per file/location/vhost etc, so that
wouldn't count
http://httpd.apache.org/docs/2.2/mod/core.html
> I might be wrong here: But if my app allows file uploads and I set this
value
> to be equivalent to 1GB (I have big files ;)) I could still easily DOS
the server;
> in my opinion it would make more sense if that would limit post data
excluding
> file data.
Ah, now I remember what was the idea. You are right. The idea is that
FORM_DATA_MAX_FIELDS_TOTAL_LENGTH includes only the length of the form-
fields
in the "multipart/form-data" POST. (A "multipart/form-data" POST may
contain file
uploads and form fields).
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:22>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by edevil):
I just like to add that the fact that Django does not deal well with large
form fields or multipart parts is an unexpected behaviour. Looking at the
file upload features one would think that Django could cope well in use
cases where we want to receive large file uploads, and so have our web
server configured to allow large request bodies. In the case of
"application/x-www-form-urlencoded" requests, large bodies should not be
expected and normally we can filter them at the web server. But "multipart
/form-data" requests are processed by the application, since we want to
allow large FILE parts but not large FIELD parts, and it so it is there
that safety measures need to be.
I know of several other sites that use Django, process large file uploads,
which are susceptible to this issue. I've contacted the developers and
they were indeed just as surprised as I was by this "feature" which I
consider a security problem.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:23>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by apollo13):
@epandurski: Can you change your patch to include the three settings you
proposed and open a PR?
@edevil: I am having a hard time to figure out what you are trying to
achieve with your comment. Since this was publicly reported we can't hide
it, no matter if we consider it a security problem or not. The ticket is
accepted, so the next step would be to provide a working patch and put it
up for discussion.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:24>
Django
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by edevil):
@apollo13: I'm trying to get this to be considered a security problem so
that it is backported to Django 1.5.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:25>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by apollo13):
@evdevil: Even if we consider it security sensitive, if 1.7 comes out
before inclusion of this patch, it won't get fixed in 1.5. Since 1.5 is
EOL soon I'd strongly encourage you to upgrade or stick with LTS releases.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:26>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by epandurski@…):
@apollo13: I may try to do this, but I am quite busy lately. Most
probably, I will not have enough spare time to work on this soon. I expect
the work to take 0.5-2 days, but I remember that the HTTP-parsing code is
quite hairy, so it might get longer to do it right.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:27>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by tomchristie):
First pass pull request: https://github.com/django/django/pull/2403
Having a single setting limiting the maximum memory for data in a request
would address this sufficiently from my POV.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:28>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by edevil):
I have a few question regarding the pull request, which I left in the
Github page. Should I post them here?
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:29>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by tomchristie):
@edevil - No, may as well keep those comments just against the PR.
(They're not really *design* discussion stuff, just implementation
details, so...)
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:30>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by anonymous):
Any news regarding this?
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:31>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by anonymous):
This is a serious problem, and there seems to be a patch available. Can
someone from the dev team look at it please?
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:32>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: nobody
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* needs_better_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
I haven't looked very closely, but there are still some unaddressed
comments on the PR, and tests are missing.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:33>
Django
-------------------------------+------------------------------------
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* owner: nobody => timo
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:34>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: timo
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
-------------------------------+------------------------------------
Changes (by timo):
* needs_better_patch: 1 => 0
* has_patch: 0 => 1
* needs_tests: 1 => 0
Comment:
Updated [https://github.com/django/django/pull/3020 PR] with tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:35>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: timo
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* needs_better_patch: 0 => 1
Comment:
See comments on the PR.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:36>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: timo
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* needs_better_patch: 1 => 0
* has_patch: 1 => 0
Comment:
I submitted a new PR that addresses some of the comments.
https://github.com/django/django/pull/3852
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:37>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: timo
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by edevil):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:38>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: timo
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by edevil):
I've answered all the comments on the PR. Is there any additional changes
you would like me to make?
Can this be merged?
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:39>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: timo
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by timgraham):
We just had the feature freeze for 1.8 and are working on stabilizing
master so we can issue an alpha release, so this likely won't be merged
until 1.9 at the earliest.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:40>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: timo
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by edevil):
I thought the "feature freeze" did not apply to security fixes?
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:41>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: timo
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by timgraham):
I think this falls under "security hardening features." If you believe
this is a security fix that should be backported to all support versions
of Django, please use secu...@djangoproject.com rather than this ticket
tracker, as described in
[https://docs.djangoproject.com/en/dev/internals/security/ our security
policies]. Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:42>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: timo
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by edevil):
I do believe this is a security problem and I have used security@ to bring
attention to this issue. In February and March of 2014 I exchanged emails
with Russell Keith-Magee and Florian Apolloner who both asked me not to
discuss this publicly. Russell even said that the core team was
considering this as a security issue and that they were discussing how to
solve it (I'm not on the security@ list).
@tomchristie above already hints that there is a problem with the current
implementation so I'm not revealing anything new.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:43>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: timo
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by timgraham):
I'll see if I have some time next week to review this issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:44>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: timo
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* needs_better_patch: 0 => 1
Comment:
Patch needs a rebase.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:45>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: timo
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* needs_better_patch: 1 => 0
Comment:
Rebased.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:46>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: timo
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by edevil):
Anything missing?
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:47>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner: timo
Type: New feature | Status: assigned
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by edevil):
Any comments?
Something I should improve?
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:48>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* owner: timo =>
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:49>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner:
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by edevil):
Any feedback on this PR?
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:50>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner:
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
* keywords: => 1.9
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:51>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner:
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by timgraham):
* keywords: 1.9 =>
Comment:
Deferring from 1.9 as the patch is still under active review.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:52>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner:
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by timgraham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:53>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner:
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Changes (by timgraham):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:54>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner:
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by edevil):
Active review... Riiiiiight.
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:55>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner:
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by apollo13):
And comments like this are the best way to discourage anyone from
reviewing this…
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:56>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner:
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by edevil):
And timelines like these, specially on security sensitive issues, are the
best way to discourage anyone from trying to contribute to Django or even
use it in the first place...
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:57>
Django
-------------------------------+------------------------------------
Reporter: epandurski@… | Owner:
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------------
Comment (by timgraham):
[https://github.com/django/django/pull/6447 Updated PR]. I've put out
another call for reviews on [https://groups.google.com/d/topic/django-
developers/UwkkZ4h0dVA/discussion django-developers].
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:55>
Django
Reporter: epandurski@… | Owner:
Type: New feature | Status: new
Component: HTTP handling | Version: master
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:56>
Django
-------------------------------------+-------------------------------------
| Graham <timograham@…>
Type: New feature | Status: closed
Component: HTTP handling | Version: master
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* owner: => Tim Graham <timograham@…>
* resolution: => fixed
Comment:
In [changeset:"929684d6ee0efb5afad51dc584489d0437d2451b" 929684d6]:
{{{
#!CommitTicketReference repository=""
revision="929684d6ee0efb5afad51dc584489d0437d2451b"
Fixed #21231 -- Enforced a max size for GET/POST values read into memory.
Thanks Tom Christie for review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:57>
Django
-------------------------------------+-------------------------------------
Reporter: epandurski@… | Owner: Tim
| Graham <timograham@…>
Type: New feature | Status: closed
Component: HTTP handling | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"fd209f62f1d83233cc634443cfac5ee4328d98b8" fd209f62]:
{{{
#!CommitTicketReference repository=""
revision="fd209f62f1d83233cc634443cfac5ee4328d98b8"
Refs #21231 -- Backport urllib.parse.parse_qsl() from Python 3.8.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:58>
Django
-------------------------------------+-------------------------------------
Reporter: epandurski@… | Owner: Tim
| Graham <timograham@…>
Type: New feature | Status: closed
Component: HTTP handling | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by GitHub <noreply@…>):
In [changeset:"83dea65ed649b5e80111017cd5c476c636872b2d" 83dea65e]:
{{{
#!CommitTicketReference repository=""
revision="83dea65ed649b5e80111017cd5c476c636872b2d"
Refs #21231 -- Corrected parse_qsl() fallback.
An oversight in fd209f62f1d83233cc634443cfac5ee4328d98b8.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21231#comment:59>