Replace insecure md5 hash generation in Order.verification_hash .

Use django.core.signing instead of our own md5 hash.

Fixes #2667.

src/oscar/apps/customer/app.py

@@ -96,7 +96,7 @@ class CustomerApplication(Application):
             url(r'^orders/$',
                 login_required(self.order_history_view.as_view()),
                 name='order-list'),
-            url(r'^order-status/(?P<order_number>[\w-]*)/(?P<hash>\w+)/$',
+            url(r'^order-status/(?P<order_number>[\w-]*)/(?P<hash>[A-z0-9-_=:]+)/$',
                 self.anon_order_detail_view.as_view(), name='anon-order'),
             url(r'^orders/(?P<order_number>[\w-]*)/$',
                 login_required(self.order_detail_view.as_view()),

src/oscar/apps/customer/views.py

@@ -604,7 +604,7 @@ class AnonymousOrderDetailView(generic.DetailView):
         # Check URL hash matches that for order to prevent spoof attacks
         order = get_object_or_404(self.model, user=None,
                                   number=self.kwargs['order_number'])
-        if self.kwargs['hash'] != order.verification_hash():
+        if not order.check_verification_hash(self.kwargs['hash']):
             raise http.Http404()
         return order
 

src/oscar/apps/order/abstract_models.py

@@ -1,11 +1,12 @@
-import hashlib
 from collections import OrderedDict
 from decimal import Decimal as D
 
 from django.conf import settings
+from django.core.signing import BadSignature, Signer
 from django.db import models
 from django.db.models import Sum
 from django.utils import timezone
+from django.utils.crypto import constant_time_compare
 from django.utils.encoding import python_2_unicode_compatible
 from django.utils.timezone import now
 from django.utils.translation import ugettext_lazy as _
@@ -299,9 +300,21 @@ class AbstractOrder(models.Model):
         return u"#%s" % (self.number,)
 
     def verification_hash(self):
-        key = '%s%s' % (self.number, settings.SECRET_KEY)
-        hash = hashlib.md5(key.encode('utf8'))
-        return hash.hexdigest()
+        signer = Signer(salt='oscar.apps.order.Order')
+        return signer.sign(self.number)
+
+    def check_verification_hash(self, hash_to_check):
+        """
+        Checks the received verification hash against this order number.
+        Returns False if the verification failed, True otherwise.
+        """
+        signer = Signer(salt='oscar.apps.order.Order')
+        try:
+            signed_number = signer.unsign(hash_to_check)
+        except BadSignature:
+            return False
+
+        return constant_time_compare(signed_number, self.number)
 
     @property
     def email(self):

tests/integration/order/test_models.py

@@ -1,7 +1,7 @@
 from datetime import timedelta, datetime
 from decimal import Decimal as D
 
-from django.test import TestCase
+from django.test import TestCase, override_settings
 from django.utils import timezone
 from django.utils.translation import ugettext_lazy as _
 import mock
@@ -391,3 +391,24 @@ class OrderTests(TestCase):
         # Set second line to returned
         event_2.line_quantities.create(line=line_2, quantity=1)
         self.assertEqual(order.shipping_status, _('Returned'))
+
+    @override_settings(SECRET_KEY='order_hash_secret')
+    def test_verification_hash_generation(self):
+        order = OrderFactory(number='111000')
+        self.assertEqual(order.verification_hash(), '111000:UJrZWNPLsq7zf1r17c3v1Q6DUmE')
+
+    @override_settings(SECRET_KEY='order_hash_secret')
+    def test_check_verification_hash_valid(self):
+        order = OrderFactory(number='111000')
+        self.assertTrue(order.check_verification_hash('111000:UJrZWNPLsq7zf1r17c3v1Q6DUmE'))
+
+    @override_settings(SECRET_KEY='order_hash_secret')
+    def test_check_verification_hash_invalid_signature(self):
+        order = OrderFactory(number='111000')
+        self.assertFalse(order.check_verification_hash('111000:HKDZWNPLsq7589517c3v1Q6DHKD'))
+
+    @override_settings(SECRET_KEY='order_hash_secret')
+    def test_check_verification_hash_valid_signature_but_wrong_number(self):
+        order = OrderFactory(number='111000')
+        # Hash is valid, but it is for a different order number
+        self.assertFalse(order.check_verification_hash('222000:knvoMB1KAiJu8meWtGce00Y88j4'))