Opened 10 years ago
Closed 9 years ago
#22990 closed Cleanup/optimization (fixed)
Sensitive POST data leaks from complex variables
Reported by: | Vlastimil Zíma | Owned by: | Vlastimil Zíma |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Using @sensitive_post_parameters()
decorator does not help much, because POST data are leaked through variables which wraps the request, such as dictionaries and tuples. Since there are a lot of wrappers and decorators this is fairly common problem.
I have added temporarily a ValueError into UserAdmin.add_view
and checked the result. The password is leaked on several occasions. Look for 'secret' in the attached email.
Attachments (1)
Change History (14)
by , 10 years ago
Attachment: | sensitive-leaked.eml added |
---|
comment:2 by , 10 years ago
After working on #21098, I think investing more time in trying to make @sensitive_post_parameters()
more secure is a losing battle. We should at least document that it's not foolproof, as well as the inherent insecurity in error reporting by email.
comment:3 by , 10 years ago
Wouldn't be worth it to handle sensitive post parameters in build_request_repr
https://github.com/django/django/blob/master/django/http/request.py#L448.
comment:4 by , 10 years ago
Perhaps; I'm not sure much complexity it would add. Are you interested in writing a patch?
comment:5 by , 10 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Bug → Cleanup/optimization |
comment:7 by , 10 years ago
Has patch: | set |
---|
I finally created a patch which solves this issue, pull request is at github: https://github.com/django/django/pull/3145
I've found a serious flaw in the tests of the technical 500 error. The tests directly called the view which rendered technical 500 page itself. This greatly cut down the traceback to single frame, thus hiding potential leakage occuring on higher levels in production environments. So, I've refactored the tests to use test client and common handling of unhandled exception.
Further issues:
- Some important data may be hidden in DEBUG mode. I'll investigate it further after this ticket is completed.
- Because of the complexity of the problem, data can't be marked as sensitive only for one channel (e.g. HTML error mails) and affected values should be generally hidden in all cases. IMHO
EXCEPTION_REPORTER_FILTER
should be replaced withSANITIZER
to remove any confusion which may occur from seeing exception reporter filter in request POST representation. - Test client may have an attribute to disable reraising the exception from the view.
comment:8 by , 10 years ago
Patch needs improvement: | set |
---|
comment:9 by , 10 years ago
Owner: | changed from | to
---|
comment:10 by , 9 years ago
Owner: | changed from | to
---|
Finally I had a bit of time to update the patch. It is in new PR https://github.com/django/django/pull/4947.
comment:11 by , 9 years ago
Patch needs improvement: | unset |
---|
comment:12 by , 9 years ago
Patch needs improvement: | set |
---|
Error email with sensitive data