#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 django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
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.
