Sanitize return_url (#6909)

CHANGELOG.txt

@@ -21,6 +21,7 @@ Changelog
  * Fix: `{% include_block with context %}` now passes local variables into the block template (Jonny Scholes)
  * Fix: Fix pagination on 'view users in a group' (Sagar Agarwal)
  * Fix: Prevent page privacy menu from being triggered by pressing enter on a char field (Sagar Agarwal)
+ * Fix: Validate host/scheme of return URLs on password authentication forms (Susan Dreher)
 
 
 2.12.3 (05.03.2021)

CONTRIBUTORS.rst

@@ -501,6 +501,7 @@ Contributors
 * Tibor Leupold
 * Joan Eliot
 * Sagar Agarwal
+* Susan Dreher
 
 Translators
 ===========

docs/releases/2.13.rst

@@ -40,6 +40,7 @@ Bug fixes
 * Make image chooser "Select format" fields translatable (Helen Chapman, Thibaud Colas)
 * Fix pagination on 'view users in a group' (Sagar Agarwal)
 * Prevent page privacy menu from being triggered by pressing enter on a char field (Sagar Agarwal)
+* Validate host/scheme of return URLs on password authentication forms (Susan Dreher)
 
 
 Upgrade considerations

wagtail/core/tests/test_page_privacy.py

@@ -48,6 +48,16 @@ class TestPagePrivacy(TestCase, WagtailTestUtils):
         response = self.client.get('/secret-plans/')
         self.assertEqual(response.templates[0].name, 'tests/simple_page.html')
 
+        self.client.logout()
+
+        # posting an invalid return_url will redirect to default login redirect
+        with self.settings(LOGIN_REDIRECT_URL='/'):
+            response = self.client.post(submit_url, {
+                'password': 'swordfish',
+                'return_url': 'https://invaliddomain.com',
+            })
+            self.assertRedirects(response, '/')
+
     def test_view_restrictions_apply_to_subpages(self):
         underpants_page = Page.objects.get(url_path='/home/secret-plans/steal-underpants/')
         response = self.client.get('/secret-plans/steal-underpants/')

wagtail/core/views.py

@@ -1,3 +1,4 @@
+from django.conf import settings
 from django.http import Http404, HttpResponse
 from django.shortcuts import get_object_or_404, redirect
 from django.urls import reverse
@@ -7,6 +8,12 @@ from wagtail.core.forms import PasswordViewRestrictionForm
 from wagtail.core.models import Page, PageViewRestriction, Site
 
 
+try:
+    from django.utils.http import url_has_allowed_host_and_scheme
+except ImportError:  # fallback for Django 2.2
+    from django.utils.http import is_safe_url as url_has_allowed_host_and_scheme
+
+
 def serve(request, path):
     # we need a valid Site object corresponding to this request in order to proceed
     site = Site.find_for_request(request)
@@ -35,9 +42,13 @@ def authenticate_with_password(request, page_view_restriction_id, page_id):
     if request.method == 'POST':
         form = PasswordViewRestrictionForm(request.POST, instance=restriction)
         if form.is_valid():
-            restriction.mark_as_passed(request)
+            return_url = form.cleaned_data['return_url']
 
-            return redirect(form.cleaned_data['return_url'])
+            if not url_has_allowed_host_and_scheme(return_url, request.get_host(), request.is_secure()):
+                return_url = settings.LOGIN_REDIRECT_URL
+
+            restriction.mark_as_passed(request)
+            return redirect(return_url)
     else:
         form = PasswordViewRestrictionForm(instance=restriction)
 

wagtail/documents/tests/test_collection_privacy.py

@@ -72,6 +72,16 @@ class TestCollectionPrivacyDocument(TestCase, WagtailTestUtils):
         # now requests to the documents url should pass authentication
         response = self.client.get(doc_url)
 
+        self.client.logout()
+
+        # posting an invalid return_url will redirect to default login redirect
+        with self.settings(LOGIN_REDIRECT_URL='/'):
+            response = self.client.post(submit_url, {
+                'password': 'swordfish',
+                'return_url': 'https://invaliddomain.com',
+            })
+            self.assertRedirects(response, '/')
+
     def test_group_restriction_with_anonymous_user(self):
         response, url = self.get_document(self.group_collection)
         self.assertRedirects(response, '/_util/login/?next={}'.format(url))

wagtail/documents/views/serve.py

@@ -17,6 +17,12 @@ from wagtail.utils import sendfile_streaming_backend
 from wagtail.utils.sendfile import sendfile
 
 
+try:
+    from django.utils.http import url_has_allowed_host_and_scheme
+except ImportError:  # fallback for Django 2.2
+    from django.utils.http import is_safe_url as url_has_allowed_host_and_scheme
+
+
 def document_etag(request, document_id, document_filename):
     Document = get_document_model()
     if hasattr(Document, 'file_hash'):
@@ -120,9 +126,13 @@ def authenticate_with_password(request, restriction_id):
     if request.method == 'POST':
         form = PasswordViewRestrictionForm(request.POST, instance=restriction)
         if form.is_valid():
-            restriction.mark_as_passed(request)
+            return_url = form.cleaned_data['return_url']
 
-            return redirect(form.cleaned_data['return_url'])
+            if not url_has_allowed_host_and_scheme(return_url, request.get_host(), request.is_secure()):
+                return_url = settings.LOGIN_REDIRECT_URL
+
+            restriction.mark_as_passed(request)
+            return redirect(return_url)
     else:
         form = PasswordViewRestrictionForm(instance=restriction)