Opened 12 months ago
Closed 12 months ago
#34968 closed Cleanup/optimization (fixed)
MultiPartParser silent large header fields size failures
Reported by: | Standa Opichal | Owned by: | Standa Opichal |
---|---|---|---|
Component: | HTTP handling | Version: | 4.2 |
Severity: | Normal | Keywords: | |
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 (last modified by )
The MultiPartParser
silently ignores parts of which the http header fields exceed 1024 bytes. This causes file uploads to 'ignore' the attached file without receiving any type of error or exception.
This is caused by the 1024 value being hardcoded here https://github.com/django/django/blob/main/django/http/multipartparser.py#L743
Here is a common http header fields limits across popular web servers (from https://stackoverflow.com/a/60623751/2448773):
- Apache - 8K
- Nginx - 4K-8K
- IIS - 8K-16K
- Tomcat - 8K – 48K
- Node (<13) - 8K; (>13) - 16K
Also reported at https://stackoverflow.com/questions/70572148/django-silently-discarding-uploaded-files-with-long-paths
Change History (9)
comment:1 by , 12 months ago
Description: | modified (diff) |
---|
comment:2 by , 12 months ago
Description: | modified (diff) |
---|
comment:3 by , 12 months ago
comment:4 by , 12 months ago
I wonder how niche your use case is as it has worked this way since the beginning (d725cc9734272f867d41f7236235c28b3931a1b2).
Indeed. We have seen it in production where our client had tried to upload files using Postman which includes also the unicode version of Content-Disposition filename which was more than 240 characters long effectively doubling the size of the header line itself which made it fail:
Content-Disposition: form-data; name="content"; filename="test.txt" filename*=UTF-8'test.txt'
Maybe we could use a module constant for this 🤔 e.g. django.http.multipartparser.MAX_HTTP_HEADER_LENGTH and set it initially to 1024.
Of course, going to adjust the PR.
The name you're proposing seems like it could be confused with a single header line length limit.
What about django.http.multipartparser.MAX_TOTAL_HEADER_SIZE
(taken from https://github.com/openstack-archive/deb-python-eventlet/blob/master/eventlet/wsgi.py and also https://support.oracle.com/knowledge/Middleware/2302288_1.html)?
comment:6 by , 12 months ago
Owner: | changed from | to
---|---|
Patch needs improvement: | set |
Status: | new → assigned |
Triage Stage: | Unreviewed → Accepted |
Type: | Bug → Cleanup/optimization |
comment:7 by , 12 months ago
Can it create a DoS vector attack?
If the limit is changed to be higher the amount of memory necessary to parse each message part is going to double and it would also extend the time to process as it tries to start with 1024 and doubles the header_end
lookahead chunk every time it doesn't find any.
The PR has been modified to stay on previous 1024 bytes with a module level constant so the change by itself doesn't pose a threat.
comment:8 by , 12 months ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
Thanks for the report. I wonder how niche your use case is as it has worked this way since the beginning (d725cc9734272f867d41f7236235c28b3931a1b2). Can it create a DoS vector attack? Maybe we could use a module constant for this 🤔 e.g.
django.http.multipartparser.MAX_HTTP_HEADER_LENGTH
and set it initially to 1024.