From 2daada800f8e28cc1ba664b3008efaefab8fb570 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Tue, 17 Sep 2013 07:46:38 -0400 Subject: [PATCH] Fixed #21098 -- Applied sensitive_post_parameters to MultiValueDict Thanks simonpercivall for the report and bmispelon for the review. --- django/views/debug.py | 37 ++++++++++++++++++++++------ tests/view_tests/tests/test_debug.py | 16 +++++++++++- tests/view_tests/views.py | 13 ++++++++++ 3 files changed, 57 insertions(+), 9 deletions(-) diff --git a/django/views/debug.py b/django/views/debug.py index 6a0f81876e..ffb343f12b 100644 --- a/django/views/debug.py +++ b/django/views/debug.py @@ -11,6 +11,7 @@ from django.http import (HttpResponse, HttpResponseServerError, HttpResponseNotFound, HttpRequest, build_request_repr) from django.template import Template, Context, TemplateDoesNotExist from django.template.defaultfilters import force_escape, pprint +from django.utils.datastructures import MultiValueDict from django.utils.html import escape from django.utils.encoding import force_bytes, smart_text from django.utils.module_loading import import_by_path @@ -118,6 +119,20 @@ class SafeExceptionReporterFilter(ExceptionReporterFilter): """ return settings.DEBUG is False + def get_cleansed_multivaluedict(self, request, multivaluedict): + """ + Replaces the keys in a MultiValueDict marked as sensitive with stars. + This mitigates leaking sensitive POST parameters if something like + request.POST['nonexistent_key'] throws an exception (#21098). + """ + sensitive_post_parameters = getattr(request, 'sensitive_post_parameters', []) + if self.is_active(request) and sensitive_post_parameters: + multivaluedict = multivaluedict.copy() + for param in sensitive_post_parameters: + if param in multivaluedict: + multivaluedict[param] = CLEANSED_SUBSTITUTE + return multivaluedict + def get_post_parameters(self, request): """ Replaces the values of POST parameters marked as sensitive with @@ -143,6 +158,15 @@ class SafeExceptionReporterFilter(ExceptionReporterFilter): else: return request.POST + def cleanse_special_types(self, request, value): + if isinstance(value, HttpRequest): + # Cleanse the request's POST parameters. + value = self.get_request_repr(value) + elif isinstance(value, MultiValueDict): + # Cleanse MultiValueDicts (request.POST is the one we usually care about) + value = self.get_cleansed_multivaluedict(request, value) + return value + def get_traceback_frame_variables(self, request, tb_frame): """ Replaces the values of variables marked as sensitive with @@ -173,17 +197,14 @@ class SafeExceptionReporterFilter(ExceptionReporterFilter): for name, value in tb_frame.f_locals.items(): if name in sensitive_variables: value = CLEANSED_SUBSTITUTE - elif isinstance(value, HttpRequest): - # Cleanse the request's POST parameters. - value = self.get_request_repr(value) + else: + value = self.cleanse_special_types(request, value) cleansed[name] = value else: - # Potentially cleanse only the request if it's one of the frame variables. + # Potentially cleanse the request and any MultiValueDicts if they + # are 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[name] = value + cleansed[name] = self.cleanse_special_types(request, value) if (tb_frame.f_code.co_name == 'sensitive_variables_wrapper' and 'sensitive_variables_wrapper' in tb_frame.f_locals): diff --git a/tests/view_tests/tests/test_debug.py b/tests/view_tests/tests/test_debug.py index 618e2175a5..c29b5520d5 100644 --- a/tests/view_tests/tests/test_debug.py +++ b/tests/view_tests/tests/test_debug.py @@ -24,7 +24,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, - sensitive_args_function_caller, sensitive_kwargs_function_caller) + sensitive_args_function_caller, sensitive_kwargs_function_caller, + multivalue_dict_key_error) @override_settings(DEBUG=True, TEMPLATE_DEBUG=True) @@ -511,6 +512,19 @@ class ExceptionReporterFilterTests(TestCase, ExceptionReportTestMixin): self.verify_paranoid_response(paranoid_view) self.verify_paranoid_email(paranoid_view) + def test_multivalue_dict_key_error(self): + """ + #21098 -- Ensure that sensitive POST parameters cannot be seen in the + error reports for if request.POST['nonexistent_key'] throws an error. + """ + with self.settings(DEBUG=True): + self.verify_unsafe_response(multivalue_dict_key_error) + self.verify_unsafe_email(multivalue_dict_key_error) + + with self.settings(DEBUG=False): + self.verify_safe_response(multivalue_dict_key_error) + self.verify_safe_email(multivalue_dict_key_error) + def test_custom_exception_reporter_filter(self): """ Ensure that it's possible to assign an exception reporter filter to diff --git a/tests/view_tests/views.py b/tests/view_tests/views.py index 814931a717..6e8fa7f813 100644 --- a/tests/view_tests/views.py +++ b/tests/view_tests/views.py @@ -289,3 +289,16 @@ class Klass(object): def sensitive_method_view(request): return Klass().method(request) + + +@sensitive_variables('sauce') +@sensitive_post_parameters('bacon-key', 'sausage-key') +def multivalue_dict_key_error(request): + cooked_eggs = ''.join(['s', 'c', 'r', 'a', 'm', 'b', 'l', 'e', 'd']) + sauce = ''.join(['w', 'o', 'r', 'c', 'e', 's', 't', 'e', 'r', 's', 'h', 'i', 'r', 'e']) + try: + request.POST['bar'] + except Exception: + exc_info = sys.exc_info() + send_log(request, exc_info) + return technical_500_response(request, *exc_info)