Commit 9180146d authored by Julien Phalip's avatar Julien Phalip
Browse files

Fixed #19453 -- Ensured that the decorated function's arguments are obfuscated...

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.
parent acc5396e
Loading
Loading
Loading
Loading
+15 −7
Original line number Diff line number Diff line
@@ -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):
    """
+2 −2
Original line number Diff line number Diff line
@@ -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

+14 −0
Original line number Diff line number Diff line
@@ -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
+73 −19
Original line number Diff line number Diff line
@@ -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):
    """
+33 −0
Original line number Diff line number Diff line
@@ -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.