From 0d71349773f8d2e8160d76b5c91a1896ece772db Mon Sep 17 00:00:00 2001 From: David Wolever Date: Mon, 9 Jun 2014 18:15:21 -0400 Subject: [PATCH] Fixed #22804 -- Added warning for unsafe value of 'sep' in Signer Thanks Jaap Roes for completing the patch. --- django/core/signing.py | 10 +++++++++- docs/internals/deprecation.txt | 3 +++ docs/releases/1.9.txt | 3 +++ tests/signing/tests.py | 18 ++++++++++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/django/core/signing.py b/django/core/signing.py index d46a386c6d..61b43316db 100644 --- a/django/core/signing.py +++ b/django/core/signing.py @@ -38,15 +38,20 @@ from __future__ import unicode_literals import base64 import datetime import json +import re import time +import warnings import zlib from django.conf import settings from django.utils import baseconv from django.utils.crypto import constant_time_compare, salted_hmac +from django.utils.deprecation import RemovedInDjango110Warning from django.utils.encoding import force_bytes, force_str, force_text from django.utils.module_loading import import_string +_SEP_UNSAFE = re.compile(r'^[A-z0-9-_=]*$') + class BadSignature(Exception): """ @@ -150,8 +155,11 @@ class Signer(object): def __init__(self, key=None, sep=':', salt=None): # Use of native strings in all versions of Python - self.sep = force_str(sep) self.key = key or settings.SECRET_KEY + self.sep = force_str(sep) + if _SEP_UNSAFE.match(self.sep): + warnings.warn('Unsafe Signer separator: %r (cannot be empty or consist of only A-z0-9-_=)' % sep, + RemovedInDjango110Warning) self.salt = force_str(salt or '%s.%s' % (self.__class__.__module__, self.__class__.__name__)) diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 67f2982292..63171d09b5 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -247,6 +247,9 @@ details on these changes. * Support for the syntax of ``{% cycle %}`` that uses comma-separated arguments will be removed. +* The warning that :class:`~django.core.signing.Signer` issues when given an + invalid separator will become an exception. + .. _deprecation-removed-in-1.9: 1.9 diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index b5c515db09..4ad98e05a2 100644 --- a/docs/releases/1.9.txt +++ b/docs/releases/1.9.txt @@ -974,6 +974,9 @@ Miscellaneous ``django.utils.feedgenerator.RssFeed.mime_type`` attributes are deprecated in favor of ``content_type``. +* :class:`~django.core.signing.Signer` now issues a warning if an invalid + separator is used. This will become an exception in Django 1.10. + .. removed-features-1.9: Features removed in 1.9 diff --git a/tests/signing/tests.py b/tests/signing/tests.py index 80dadee7f4..a8de1b0cbc 100644 --- a/tests/signing/tests.py +++ b/tests/signing/tests.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals import datetime +import warnings from django.core import signing from django.test import SimpleTestCase @@ -112,6 +113,23 @@ class TestSigner(SimpleTestCase): s = signing.Signer(binary_key) self.assertEqual('foo:6NB0fssLW5RQvZ3Y-MTerq2rX7w', s.sign('foo')) + def test_valid_sep(self): + separators = ['/', '*sep*', ','] + for sep in separators: + signer = signing.Signer('predictable-secret', sep=sep) + self.assertEqual('foo%ssH9B01cZcJ9FoT_jEVkRkNULrl8' % sep, signer.sign('foo')) + + def test_invalid_sep(self): + """should warn on invalid separator""" + separators = ['', '-', 'abc'] + for sep in separators: + with warnings.catch_warnings(record=True) as recorded: + warnings.simplefilter('always') + signing.Signer(sep=sep) + self.assertEqual(len(recorded), 1) + msg = str(recorded[0].message) + self.assertTrue(msg.startswith('Unsafe Signer separator')) + class TestTimestampSigner(SimpleTestCase):