[Django] #22990: Sensitive POST data leaks from complex variables
Django
------------------------------+--------------------
Reporter: vzima | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+--------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22990>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: vzima | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by vzima):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
This is most likely a generic version of #21098.
--
Ticket URL: <https://code.djangoproject.com/ticket/22990#comment:1>
Django
Reporter: vzima | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timo):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22990#comment:2>
Django
Reporter: vzima | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by vzima):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22990#comment:3>
Django
Reporter: vzima | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timo):
Perhaps; I'm not sure much complexity it would add. Are you interested in
writing a patch?
--
Ticket URL: <https://code.djangoproject.com/ticket/22990#comment:4>
Django
Reporter: vzima | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Core (Other) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* type: Bug => Cleanup/optimization
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/22990#comment:5>
Django
Reporter: vzima | Owner: vzima
Type: Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by vzima):
* status: new => assigned
* owner: nobody => vzima
Comment:
I'll give it a try.
--
Ticket URL: <https://code.djangoproject.com/ticket/22990#comment:6>
Django
Reporter: vzima | Owner: vzima
Type: Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by vzima):
* has_patch: 0 => 1
Comment:
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 with `SANITIZER` 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/22990#comment:7>
Django
Reporter: vzima | Owner: vzima
Type: Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by berkerpeksag):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/22990#comment:8>
Django
Reporter: vzima | Owner: auvipy
Type: Cleanup/optimization | Status: assigned
Component: Core (Other) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by auvipy):
* owner: vzima => auvipy
--
Ticket URL: <https://code.djangoproject.com/ticket/22990#comment:9>
Django
Reporter: vzima | Owner: vzima
Type: Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by vzima):
* owner: auvipy => vzima
Comment:
Finally I had a bit of time to update the patch. It is in new PR
https://github.com/django/django/pull/4947.
--
Ticket URL: <https://code.djangoproject.com/ticket/22990#comment:10>
Django
Reporter: vzima | Owner: vzima
Type: Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by vzima):
* needs_better_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/22990#comment:11>
Django
Reporter: vzima | Owner: vzima
Type: Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/22990#comment:12>
Django
Reporter: vzima | Owner: vzima
Component: Core (Other) | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by timgraham):
* status: assigned => closed
* resolution: => fixed
Comment:
Fixed by #25099
--
Ticket URL: <https://code.djangoproject.com/ticket/22990#comment:13>