Permalink
Browse files

[1.6.x] Fixed #21098 -- Applied sensitive_post_parameters to MultiVal…

…ueDict

Thanks simonpercivall for the report and bmispelon for the review.

Backport of 2daada8 from master
  • Loading branch information...
1 parent dbad65d commit 778d4da9cc249839b059e45d90e3a947bb76dd6d @timgraham timgraham committed Sep 17, 2013
View
@@ -11,6 +11,7 @@
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 @@ def is_active(self, request):
"""
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 @@ def get_post_parameters(self, request):
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 @@ def get_traceback_frame_variables(self, request, tb_frame):
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):
@@ -22,7 +22,8 @@
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)
from django.utils.unittest import skipIf
@@ -495,6 +496,19 @@ def test_paranoid_request(self):
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
View
@@ -272,3 +272,16 @@ def method(self, request):
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)

3 comments on commit 778d4da

Member

charettes replied Sep 19, 2013

Looks like there was an erronously commited .pyc file.

Owner

timgraham replied Sep 19, 2013

Thanks, must have been running tests while committing. Does it seem reasonable to add __pycache__ to .gitignore?

Member

charettes replied Sep 19, 2013

I guess this a sane project specific ignore pattern since *.py[co] is already there.

Please sign in to comment.