#263 Fix vunerability in views

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 1.8.0
+
+  - #263 Fix vunerability in views
+
 ## 1.7.0
 
   - Added support for Django 3.2 and Django 4.0

notifications/tests/settings.py

@@ -77,3 +77,5 @@
     INSTALLED_APPS.append('notifications.tests.sample_notifications')
     NOTIFICATIONS_NOTIFICATION_MODEL = 'sample_notifications.Notification'
     TEMPLATES[0]['DIRS'] += [os.path.join(BASE_DIR, '../templates')]
+
+ALLOWED_HOSTS = []

notifications/tests/tests.py

@@ -14,7 +14,7 @@
 from django.core.exceptions import ImproperlyConfigured
 from django.db import connection
 from django.template import Context, Template
-from django.test import RequestFactory, TestCase
+from django.test import Client, RequestFactory, TestCase
 from django.test.utils import CaptureQueriesContext
 from django.utils import timezone
 from django.utils.timezone import localtime, utc
@@ -39,7 +39,13 @@
     # Django <= 1.6
     from django.core.urlresolvers import reverse  # pylint: disable=no-name-in-module,import-error
 
-
+MALICIOUS_NEXT_URLS = [
+    "http://bla.com",
+    "http://www.bla.com",
+    "https://bla.com",
+    "https://www.bla.com",
+    "ftp://www.bla.com/file.exe",
+]
 
 class NotificationTest(TestCase):
     ''' Django notifications automated tests '''
@@ -250,6 +256,17 @@ def test_next_pages(self):
         })
         self.assertRedirects(response, reverse('notifications:unread') + query_parameters)
 
+    @override_settings(ALLOWED_HOSTS=["www.notifications.com"])
+    def test_malicious_next_pages(self):
+        self.client.force_login(self.to_user)
+        query_parameters = '?var1=hello&var2=world'
+
+        for next_url in MALICIOUS_NEXT_URLS:
+            response = self.client.get(reverse('notifications:mark_all_as_read'),data={
+                "next": next_url  + query_parameters,
+            })
+            self.assertRedirects(response, reverse('notifications:unread'))
+
     def test_delete_messages_pages(self):
         self.login('to', 'pwd')
 

notifications/views.py

@@ -4,14 +4,16 @@
     StrictVersion  # pylint: disable=no-name-in-module,import-error
 
 from django import get_version
+from django.conf import settings
 from django.contrib.auth.decorators import login_required
 from django.forms import model_to_dict
 from django.shortcuts import get_object_or_404, redirect
 from django.utils.decorators import method_decorator
+from django.utils.encoding import iri_to_uri
+from django.utils.http import url_has_allowed_host_and_scheme
 from django.views.decorators.cache import never_cache
 from django.views.generic import ListView
-from notifications import settings
-from notifications.settings import get_config
+from notifications import settings as notification_settings
 from notifications.utils import id2slug, slug2id
 from swapper import load_model
 
@@ -36,7 +38,7 @@ def JsonResponse(data):  # noqa
 class NotificationViewList(ListView):
     template_name = 'notifications/list.html'
     context_object_name = 'notifications'
-    paginate_by = settings.get_config()['PAGINATE_BY']
+    paginate_by = notification_settings.get_config()['PAGINATE_BY']
 
     @method_decorator(login_required)
     def dispatch(self, request, *args, **kwargs):
@@ -50,7 +52,7 @@ class AllNotificationsList(NotificationViewList):
     """
 
     def get_queryset(self):
-        if settings.get_config()['SOFT_DELETE']:
+        if notification_settings.get_config()['SOFT_DELETE']:
             qset = self.request.user.notifications.active()
         else:
             qset = self.request.user.notifications.all()
@@ -69,8 +71,8 @@ def mark_all_as_read(request):
 
     _next = request.GET.get('next')
 
-    if _next:
-        return redirect(_next)
+    if _next and url_has_allowed_host_and_scheme(_next, settings.ALLOWED_HOSTS):
+        return redirect(iri_to_uri(_next))
     return redirect('notifications:unread')
 
 
@@ -84,8 +86,8 @@ def mark_as_read(request, slug=None):
 
     _next = request.GET.get('next')
 
-    if _next:
-        return redirect(_next)
+    if _next and url_has_allowed_host_and_scheme(_next, settings.ALLOWED_HOSTS):
+        return redirect(iri_to_uri(_next))
 
     return redirect('notifications:unread')
 
@@ -100,8 +102,8 @@ def mark_as_unread(request, slug=None):
 
     _next = request.GET.get('next')
 
-    if _next:
-        return redirect(_next)
+    if _next and url_has_allowed_host_and_scheme(_next, settings.ALLOWED_HOSTS):
+        return redirect(iri_to_uri(_next))
 
     return redirect('notifications:unread')
 
@@ -113,16 +115,16 @@ def delete(request, slug=None):
     notification = get_object_or_404(
         Notification, recipient=request.user, id=notification_id)
 
-    if settings.get_config()['SOFT_DELETE']:
+    if notification_settings.get_config()['SOFT_DELETE']:
         notification.deleted = True
         notification.save()
     else:
         notification.delete()
 
     _next = request.GET.get('next')
 
-    if _next:
-        return redirect(_next)
+    if _next and url_has_allowed_host_and_scheme(_next, settings.ALLOWED_HOSTS):
+        return redirect(iri_to_uri(_next))
 
     return redirect('notifications:all')
 
@@ -160,7 +162,7 @@ def live_unread_notification_list(request):
         }
         return JsonResponse(data)
 
-    default_num_to_fetch = get_config()['NUM_TO_FETCH']
+    default_num_to_fetch = notification_settings.get_config()['NUM_TO_FETCH']
     try:
         # If they don't specify, make it 5.
         num_to_fetch = request.GET.get('max', default_num_to_fetch)
@@ -208,7 +210,7 @@ def live_all_notification_list(request):
         }
         return JsonResponse(data)
 
-    default_num_to_fetch = get_config()['NUM_TO_FETCH']
+    default_num_to_fetch = notification_settings.get_config()['NUM_TO_FETCH']
     try:
         # If they don't specify, make it 5.
         num_to_fetch = request.GET.get('max', default_num_to_fetch)