Opened 2 years ago
Closed 2 years 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 , 2 years ago
| Description: | modified (diff) |
|---|
comment:2 by , 2 years ago
| Description: | modified (diff) |
|---|
comment:3 by , 2 years ago
comment:4 by , 2 years 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 , 2 years ago
| Owner: | changed from to |
|---|---|
| Patch needs improvement: | set |
| Status: | new → assigned |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Bug → Cleanup/optimization |
comment:7 by , 2 years 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 , 2 years 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_LENGTHand set it initially to 1024.