From 9180146d21cf2a31eec994b4adc0e50c7120f17f Mon Sep 17 00:00:00 2001 From: Julien Phalip Date: Mon, 31 Dec 2012 09:34:08 -0800 Subject: [PATCH] Fixed #19453 -- Ensured that the decorated function's arguments are obfuscated in the @sensitive_variables decorator's frame, in case the variables associated with those arguments were meant to be obfuscated from the decorated function's frame. Thanks to vzima for the report. --- django/views/debug.py | 22 ++++-- django/views/decorators/debug.py | 4 +- docs/howto/error-reporting.txt | 14 ++++ tests/regressiontests/views/tests/debug.py | 92 +++++++++++++++++----- tests/regressiontests/views/views.py | 33 ++++++++ 5 files changed, 137 insertions(+), 28 deletions(-) diff --git a/django/views/debug.py b/django/views/debug.py index aaa7e40efe..e5f4c70191 100644 --- a/django/views/debug.py +++ b/django/views/debug.py @@ -172,13 +172,12 @@ class SafeExceptionReporterFilter(ExceptionReporterFilter): break current_frame = current_frame.f_back - cleansed = [] + cleansed = {} if self.is_active(request) and sensitive_variables: if sensitive_variables == '__ALL__': # Cleanse all variables for name, value in tb_frame.f_locals.items(): - cleansed.append((name, CLEANSED_SUBSTITUTE)) - return cleansed + cleansed[name] = CLEANSED_SUBSTITUTE else: # Cleanse specified variables for name, value in tb_frame.f_locals.items(): @@ -187,16 +186,25 @@ class SafeExceptionReporterFilter(ExceptionReporterFilter): elif isinstance(value, HttpRequest): # Cleanse the request's POST parameters. value = self.get_request_repr(value) - cleansed.append((name, value)) - return cleansed + cleansed[name] = value else: # Potentially cleanse only the request if it's one of the frame variables. for name, value in tb_frame.f_locals.items(): if isinstance(value, HttpRequest): # Cleanse the request's POST parameters. value = self.get_request_repr(value) - cleansed.append((name, value)) - return cleansed + cleansed[name] = value + + if (tb_frame.f_code.co_name == 'sensitive_variables_wrapper' + and 'sensitive_variables_wrapper' in tb_frame.f_locals): + # For good measure, obfuscate the decorated function's arguments in + # the sensitive_variables decorator's frame, in case the variables + # associated with those arguments were meant to be obfuscated from + # the decorated function's frame. + cleansed['func_args'] = CLEANSED_SUBSTITUTE + cleansed['func_kwargs'] = CLEANSED_SUBSTITUTE + + return cleansed.items() class ExceptionReporter(object): """ diff --git a/django/views/decorators/debug.py b/django/views/decorators/debug.py index 5c222963d3..78ae6b1442 100644 --- a/django/views/decorators/debug.py +++ b/django/views/decorators/debug.py @@ -26,12 +26,12 @@ def sensitive_variables(*variables): """ def decorator(func): @functools.wraps(func) - def sensitive_variables_wrapper(*args, **kwargs): + def sensitive_variables_wrapper(*func_args, **func_kwargs): if variables: sensitive_variables_wrapper.sensitive_variables = variables else: sensitive_variables_wrapper.sensitive_variables = '__ALL__' - return func(*args, **kwargs) + return func(*func_args, **func_kwargs) return sensitive_variables_wrapper return decorator diff --git a/docs/howto/error-reporting.txt b/docs/howto/error-reporting.txt index 35add32e4c..98b3b4e4d8 100644 --- a/docs/howto/error-reporting.txt +++ b/docs/howto/error-reporting.txt @@ -153,6 +153,20 @@ production environment (that is, where :setting:`DEBUG` is set to ``False``): def my_function(): ... + .. admonition:: When using mutiple decorators + + If the variable you want to hide is also a function argument (e.g. + '``user``' in the following example), and if the decorated function has + mutiple decorators, then make sure to place ``@sensible_variables`` at + the top of the decorator chain. This way it will also hide the function + argument as it gets passed through the other decorators:: + + @sensitive_variables('user', 'pw', 'cc') + @some_decorator + @another_decorator + def process_info(user): + ... + .. function:: sensitive_post_parameters(*parameters) If one of your views receives an :class:`~django.http.HttpRequest` object diff --git a/tests/regressiontests/views/tests/debug.py b/tests/regressiontests/views/tests/debug.py index 4fdaad5010..0e36948b98 100644 --- a/tests/regressiontests/views/tests/debug.py +++ b/tests/regressiontests/views/tests/debug.py @@ -7,7 +7,6 @@ import inspect import os import sys -from django.conf import settings from django.core import mail from django.core.files.uploadedfile import SimpleUploadedFile from django.core.urlresolvers import reverse @@ -19,7 +18,8 @@ from django.views.debug import ExceptionReporter from .. import BrokenException, except_args from ..views import (sensitive_view, non_sensitive_view, paranoid_view, - custom_exception_reporter_filter_view, sensitive_method_view) + custom_exception_reporter_filter_view, sensitive_method_view, + sensitive_args_function_caller, sensitive_kwargs_function_caller) @override_settings(DEBUG=True, TEMPLATE_DEBUG=True) @@ -306,17 +306,28 @@ class ExceptionReportTestMixin(object): response = view(request) self.assertEqual(len(mail.outbox), 1) email = mail.outbox[0] + # Frames vars are never shown in plain text email reports. - body = force_text(email.body) - self.assertNotIn('cooked_eggs', body) - self.assertNotIn('scrambled', body) - self.assertNotIn('sauce', body) - self.assertNotIn('worcestershire', body) + body_plain = force_text(email.body) + self.assertNotIn('cooked_eggs', body_plain) + self.assertNotIn('scrambled', body_plain) + self.assertNotIn('sauce', body_plain) + self.assertNotIn('worcestershire', body_plain) + + # Frames vars are shown in html email reports. + body_html = force_text(email.alternatives[0][0]) + self.assertIn('cooked_eggs', body_html) + self.assertIn('scrambled', body_html) + self.assertIn('sauce', body_html) + self.assertIn('worcestershire', body_html) + if check_for_POST_params: for k, v in self.breakfast_data.items(): # All POST parameters are shown. - self.assertIn(k, body) - self.assertIn(v, body) + self.assertIn(k, body_plain) + self.assertIn(v, body_plain) + self.assertIn(k, body_html) + self.assertIn(v, body_html) def verify_safe_email(self, view, check_for_POST_params=True): """ @@ -328,22 +339,35 @@ class ExceptionReportTestMixin(object): response = view(request) self.assertEqual(len(mail.outbox), 1) email = mail.outbox[0] + # Frames vars are never shown in plain text email reports. - body = force_text(email.body) - self.assertNotIn('cooked_eggs', body) - self.assertNotIn('scrambled', body) - self.assertNotIn('sauce', body) - self.assertNotIn('worcestershire', body) + body_plain = force_text(email.body) + self.assertNotIn('cooked_eggs', body_plain) + self.assertNotIn('scrambled', body_plain) + self.assertNotIn('sauce', body_plain) + self.assertNotIn('worcestershire', body_plain) + + # Frames vars are shown in html email reports. + body_html = force_text(email.alternatives[0][0]) + self.assertIn('cooked_eggs', body_html) + self.assertIn('scrambled', body_html) + self.assertIn('sauce', body_html) + self.assertNotIn('worcestershire', body_html) + if check_for_POST_params: for k, v in self.breakfast_data.items(): # All POST parameters' names are shown. - self.assertIn(k, body) + self.assertIn(k, body_plain) # Non-sensitive POST parameters' values are shown. - self.assertIn('baked-beans-value', body) - self.assertIn('hash-brown-value', body) + self.assertIn('baked-beans-value', body_plain) + self.assertIn('hash-brown-value', body_plain) + self.assertIn('baked-beans-value', body_html) + self.assertIn('hash-brown-value', body_html) # Sensitive POST parameters' values are not shown. - self.assertNotIn('sausage-value', body) - self.assertNotIn('bacon-value', body) + self.assertNotIn('sausage-value', body_plain) + self.assertNotIn('bacon-value', body_plain) + self.assertNotIn('sausage-value', body_html) + self.assertNotIn('bacon-value', body_html) def verify_paranoid_email(self, view): """ @@ -445,6 +469,36 @@ class ExceptionReporterFilterTests(TestCase, ExceptionReportTestMixin): self.verify_safe_email(sensitive_method_view, check_for_POST_params=False) + def test_sensitive_function_arguments(self): + """ + Ensure that sensitive variables don't leak in the sensitive_variables + decorator's frame, when those variables are passed as arguments to the + decorated function. + Refs #19453. + """ + with self.settings(DEBUG=True): + self.verify_unsafe_response(sensitive_args_function_caller) + self.verify_unsafe_email(sensitive_args_function_caller) + + with self.settings(DEBUG=False): + self.verify_safe_response(sensitive_args_function_caller, check_for_POST_params=False) + self.verify_safe_email(sensitive_args_function_caller, check_for_POST_params=False) + + def test_sensitive_function_keyword_arguments(self): + """ + Ensure that sensitive variables don't leak in the sensitive_variables + decorator's frame, when those variables are passed as keyword arguments + to the decorated function. + Refs #19453. + """ + with self.settings(DEBUG=True): + self.verify_unsafe_response(sensitive_kwargs_function_caller) + self.verify_unsafe_email(sensitive_kwargs_function_caller) + + with self.settings(DEBUG=False): + self.verify_safe_response(sensitive_kwargs_function_caller, check_for_POST_params=False) + self.verify_safe_email(sensitive_kwargs_function_caller, check_for_POST_params=False) + class AjaxResponseExceptionReporterFilter(TestCase, ExceptionReportTestMixin): """ diff --git a/tests/regressiontests/views/views.py b/tests/regressiontests/views/views.py index ed9d61144a..748f07637f 100644 --- a/tests/regressiontests/views/views.py +++ b/tests/regressiontests/views/views.py @@ -132,6 +132,7 @@ def send_log(request, exc_info): ][0] orig_filters = admin_email_handler.filters admin_email_handler.filters = [] + admin_email_handler.include_html = True logger.error('Internal Server Error: %s', request.path, exc_info=exc_info, extra={ @@ -184,6 +185,38 @@ def paranoid_view(request): send_log(request, exc_info) return technical_500_response(request, *exc_info) +def sensitive_args_function_caller(request): + try: + sensitive_args_function(''.join(['w', 'o', 'r', 'c', 'e', 's', 't', 'e', 'r', 's', 'h', 'i', 'r', 'e'])) + except Exception: + exc_info = sys.exc_info() + send_log(request, exc_info) + return technical_500_response(request, *exc_info) + +@sensitive_variables('sauce') +def sensitive_args_function(sauce): + # Do not just use plain strings for the variables' values in the code + # so that the tests don't return false positives when the function's source + # is displayed in the exception report. + cooked_eggs = ''.join(['s', 'c', 'r', 'a', 'm', 'b', 'l', 'e', 'd']) + raise Exception + +def sensitive_kwargs_function_caller(request): + try: + sensitive_kwargs_function(''.join(['w', 'o', 'r', 'c', 'e', 's', 't', 'e', 'r', 's', 'h', 'i', 'r', 'e'])) + except Exception: + exc_info = sys.exc_info() + send_log(request, exc_info) + return technical_500_response(request, *exc_info) + +@sensitive_variables('sauce') +def sensitive_kwargs_function(sauce=None): + # Do not just use plain strings for the variables' values in the code + # so that the tests don't return false positives when the function's source + # is displayed in the exception report. + cooked_eggs = ''.join(['s', 'c', 'r', 'a', 'm', 'b', 'l', 'e', 'd']) + raise Exception + class UnsafeExceptionReporterFilter(SafeExceptionReporterFilter): """ Ignores all the filtering done by its parent class.