MultiValueDictKeyError leaks sensitive POST data

[Simon Percivall]

Getting an error in MultiValueDict on a POST, such as doing request.POST['foo'], will leak the POST data without any escaping by Django, i.e. the MultiValueDictKeyError contains an unescaped repr of request.POST, no matter if you've added for instance @sensitive_post_parameters("password").

[Tim Graham]

This seems like a tricky one. I took a stab at some initial ideas, but it's still possible to leak data with this approach.

​https://github.com/django/django/pull/1624

[Aymeric Augustin]

If I understand correctly, this ticket is about Django's debug page, which is only displayed when DEBUG = True.

sensitive_post_parameters is only supported when DEBUG = False.

Relevant excerpt from ​the error reporting docs:

Django offers a set of function decorators to help you control which information should be filtered out of error reports in a production environment (that is, where DEBUG is set to False)

I'm going to close this ticket for this reason, and for a few others I describe below.

---

Tim's first attemps shows that it will be very hard to scrub sensitive variables. They may end up anywhere in the stack trace. For instance, if a function contains hash_src = (salt + password).encode('utf-8'), and Django displays the value of local variables in that frame, the value of the password will appear on screen. It seems impossible to defend against all leaking scenarios in the context of a debug page that aims at displaying as much information as possible.

Even if it we had a strong mechanism to mask sensitive variables in the debug page (like looking at their value and replacing it with a placeholder in the generated HTML), I'm not sure it would be a good idea. It's very frustrating when a debugging tool hides the information you need. In a debug page used only in development (when DEBUG = True), it makes sense to priorize debugging capabilities. While it's annoying to read one's password on screen, the cure may be worse than the disease.

Leaking POST data in the debug page isn't much of an issue. The data was entered by a developer working on the project, and it's leaked to the same developer.

sensitive_post_parameters is designed to avoid leaking data in logs or error reports, not in generated HTML, and especially not in the debug page.

[anonymous]

I don't believe the concern here is the debug page, repr of the request, including post data, is sent by default email handler (not HTML version).

[Simon Percivall]

Yes, correct; this information is included by the AdminEmailHandler, and by any other loggers, and that is why it's a real issue.

[Tim Graham]

#15625 (removing the dictionary from the MultiValueDictKeyError message) will fix this for plain text emails sent by AdminEmailHandler.

HTML email is another matter because local variables are included which includes the self variable in MultiValueDict.__getitem__ which is the unsanitized POST dict. We may be better off adding some warnings to the documentation than trying to make this protection bulletproof as it seems exceedingly difficult to do so.

[anonymous]

After a fix for #15625, sanitizing MultiValueDict is more manageable. I'm not 100% sold it's the right thing to do since you might have other MultiValueDicts that you don't want to treat similarly, but this might be worth that tradeoff. I've updated the original ​PR.

[Tim Graham <timograham@…>]

In 2daada800f8e28cc1ba664b3008efaefab8fb570:

Fixed #21098 -- Applied sensitive_post_parameters to MultiValueDict

Thanks simonpercivall for the report and bmispelon for the review.

[Tim Graham <timograham@…>]

In 778d4da9cc249839b059e45d90e3a947bb76dd6d:

[1.6.x] Fixed #21098 -- Applied sensitive_post_parameters to MultiValueDict

Thanks simonpercivall for the report and bmispelon for the review.

Backport of 2daada800f from master