#28215: sensitive_post_parameters/sensitive_variables leaking sensitive values
into
the http 500 exception email
---------------------------------+--------------------------------------
Reporter: Peter Zsoldos | Owner: (none)
Type: Bug | Status: new
Component: Error reporting | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+--------------------------------------
Description changed by Peter Zsoldos:
Old description:
> == tl;dr
>
> despite using sensitive_xxx decorator, sensitive data can end up in the
> 500 error emails Django sends, as these decorators only protect the data
> inside the very function they are decorated
>
> == repro
>
> Is attached for all current supported Django versions (1.8, 1.10, 1.11),
> simply unpack and run `tox`
>
> The test can seem complicated due to the limitations of the test client
> in testing 500 responses - see #18707
>
> == why I think it is an issue
>
> While I'm aware of the [disclaimers in the documentation about filtering
> sensitive data (https://docs.djangoproject.com/en/1.8/howto/error-
> reporting/#custom-error-reports), because of the impact of it - even on
> users who don't explicitly use any of the `sensitive_x` decorators
> themselves, I think it is a leak that should be stopped.
>
> * typical sensitive data is passwords. We have discovered this issue due
> to a bug in our custom authentication backend. These passwords could also
> be used beyond just the single Django system - whether because of single
> sign on solutions like LDAP/active-directory, or simply because users
> might reuse their passwords across sites
> * exception emails might be sent through third party providers, which may
> keep track of the sent message body. Internal IT departments might also
> be considered such 3rd parties too.
> * support people (admins receiving 500 emails) see supposedly private
> data
>
> == potential solution ideas (which might be wrong of course :))
>
> === writing a custom exception filter
>
> * simply don't report any variables once encountered -
> https://gist.github.com/zsoldosp/5710abaa9dedc03417d60bcc714c95d4
> * keep track of protected variable names and replace those in frames
> further down the stack (i.e.: if parameter 'password', is sensitive,
> cleanse variables names 'password' too in all methods)
>
> === wrapping sensitive variables into a special object
>
> Instead of just using the sensitive data in reporting, wrap these
> variables in an object that has 'contains_sensitive_data' attribute,
> i.e.: if it is stored into another variable, as it is a 'pointer' to the
> original, it will have that attribute, and thus can be filtered out in
> the exception report.
>
> This isn't perfect either, e.g.: `password = password.strip()`, though by
> overriding a lot of methods or using `__getattr__` magic, it could work.
> Might only be 'reasonable' to do so for request parameters, as there at
> least we know the limited set of variable types we receive
>
> {{{#!python
> @sensitive_request_params
> def view(request):
> ....
>
> # inside sensitive_request_params
> for sensitive_variable_name in sensitive_variable_names:
> if sensitive_variable_name in request.POST:
> request.POST[sensitive_variable_name] =
> SensitiveVariable(request.POST[sensitive_variable_name])
> ....
> }}}
New description:
== tl;dr
despite using sensitive_xxx decorator, sensitive data can end up in the
500 error emails Django sends, as these decorators only protect the data
inside the very function they are decorated
== repro
{{{#!python
class ReproTestCase(TransactionTestCase):
def
test_when_login_view_raises_an_exception_password_is_not_in_the_500_email(self):
# noqa: E501
password = '$0m3 P4$$w0rd'
exception_email_html_body =
self.get_500_email_html_for_login_error(
username='some_user', password=password
)
self.assertNotIn(
member=password, container=exception_email_html_body)
def get_500_email_html_for_login_error(self, username, password):
# patch this methodd so AuthenticationForm.clean is
# called which has local password variable
login_view_raising_value_error = patch(
'django.contrib.auth.forms.authenticate',
side_effect=ValueError('some error')
)
self.goto_login_page()
with TestClientNotRaisingExceptionButCapturing(self.client) as
capture: # see implementation details in attachment
with login_view_raising_value_error:
self.submit_login(username=username, password=password)
request = capture.get_captured_request()
exc_type, exc_value, tb = capture.stored_exc_info
# based on django.utils.log.AdminEmailHandler.emit
reporter = ExceptionReporter(
request=request, is_email=True,
exc_type=exc_type, exc_value=exc_value, tb=tb)
self.assertTrue(reporter.filter.is_active(request))
return reporter.get_traceback_html()
}}}
Is attached for all current supported Django versions (1.8, 1.10, 1.11),
simply unpack and run `tox`
The test can seem complicated due to the limitations of the test client in
testing 500 responses - see #18707
== why I think it is an issue
While I'm aware of the [disclaimers in the documentation about filtering
sensitive data (https://docs.djangoproject.com/en/1.8/howto/error-
reporting/#custom-error-reports), because of the impact of it - even on
users who don't explicitly use any of the `sensitive_x` decorators
themselves, I think it is a leak that should be stopped.
* typical sensitive data is passwords. We have discovered this issue due
to a bug in our custom authentication backend. These passwords could also
be used beyond just the single Django system - whether because of single
sign on solutions like LDAP/active-directory, or simply because users
might reuse their passwords across sites
* exception emails might be sent through third party providers, which may
keep track of the sent message body. Internal IT departments might also be
considered such 3rd parties too.
* support people (admins receiving 500 emails) see supposedly private data
== potential solution ideas (which might be wrong of course :))
=== writing a custom exception filter
* simply don't report any variables once encountered -
https://gist.github.com/zsoldosp/5710abaa9dedc03417d60bcc714c95d4
* keep track of protected variable names and replace those in frames
further down the stack (i.e.: if parameter 'password', is sensitive,
cleanse variables names 'password' too in all methods)
=== wrapping sensitive variables into a special object
Instead of just using the sensitive data in reporting, wrap these
variables in an object that has 'contains_sensitive_data' attribute, i.e.:
if it is stored into another variable, as it is a 'pointer' to the
original, it will have that attribute, and thus can be filtered out in the
exception report.
This isn't perfect either, e.g.: `password = password.strip()`, though by
overriding a lot of methods or using `__getattr__` magic, it could work.
Might only be 'reasonable' to do so for request parameters, as there at
least we know the limited set of variable types we receive
{{{#!python
@sensitive_request_params
def view(request):
....
# inside sensitive_request_params
for sensitive_variable_name in sensitive_variable_names:
if sensitive_variable_name in request.POST:
request.POST[sensitive_variable_name] =
SensitiveVariable(request.POST[sensitive_variable_name])
....
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/28215#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/066.fc3ed4df625c530cb1ccf99662e35ea3%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
