Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#24209 closed Bug (fixed)

Incorrectly formatted Content-Disposition headers may cause error.

Reported by: Tom Christie Owned by: Raúl Cumplido
Component: HTTP handling Version: 1.8alpha1
Severity: Normal Keywords:
Cc: raulcumplido@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

MultiPartParser does not handle malformed Content-Disposition headers gracefully in some cases. If the client uses filename*= syntax, but omits the language enclosed in single quotes, then an error will be raised instead of gracefully ignoring the component.

This is introduced by the fix in https://code.djangoproject.com/ticket/22971

Here we split the header without checking if the split length is correct...

https://github.com/django/django/blob/1.8a1/django/http/multipartparser.py#L649

The has_encoding test should probably be...

if name.endswith('*') and p[i + 1:].count("'") == 2

Change History (8)

comment:1 Changed 8 years ago by Tom Christie

Summary: Incorrectly formatted Content-Disposition headers will cause error.Incorrectly formatted Content-Disposition headers may cause error.

comment:2 Changed 8 years ago by Claude Paroz

Triage Stage: UnreviewedAccepted

Sure. Do you have by chance a "real world" failing example?

comment:3 Changed 8 years ago by Raúl Cumplido

Cc: raulcumplido@… added
Has patch: set
Owner: changed from nobody to Raúl Cumplido
Status: newassigned

comment:4 Changed 8 years ago by Tom Christie

I believe omitting the quotes for the language would cause a 500.
Eg. this would work

Content-Disposition: attachment; filename*=UTF-8'en-us'filename.txt

But this would raise a 500. (If that was in the form of say, a MultiPartParseError that'd be perfectly acceptable of course. It's just the unguarded 500 that's slightly non-ideal)

Content-Disposition: attachment; filename*=UTF-8"en-us"filename.txt

comment:5 Changed 8 years ago by Claude Paroz

Raùl provided a patch (https://github.com/django/django/pull/3983). Tom, could you check if it looks ok for you?

comment:6 Changed 8 years ago by Tom Christie

Yup that looks correct.

comment:7 Changed 8 years ago by Claude Paroz <claude@…>

Resolution: fixed
Status: assignedclosed

In ac650d02cbb66ab652ae529b8f03b486ef974dfb:

Fixed #24209 -- Prevented crash when parsing malformed RFC 2231 headers

Thanks Tom Christie for the report and review.

comment:8 Changed 8 years ago by Claude Paroz <claude@…>

In 7cc1b4710ea3fbe3a076d14f9265f115a615964a:

[1.8.x] Fixed #24209 -- Prevented crash when parsing malformed RFC 2231 headers

Thanks Tom Christie for the report and review.
Backport of ac650d02cb from master.

Note: See TracTickets for help on using tickets.
Back to Top