From 09431f7b22b51e29d91fdbbe3e64f046103fafb0 Mon Sep 17 00:00:00 2001 From: Susan Dreher Date: Thu, 11 Mar 2021 18:37:43 -0500 Subject: [PATCH] Sanitize return_url (#6909) --- CHANGELOG.txt | 1 + CONTRIBUTORS.rst | 1 + docs/releases/2.13.rst | 1 + wagtail/core/tests/test_page_privacy.py | 10 ++++++++++ wagtail/core/views.py | 15 +++++++++++++-- .../documents/tests/test_collection_privacy.py | 10 ++++++++++ wagtail/documents/views/serve.py | 14 ++++++++++++-- 7 files changed, 48 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index a99ce1cfc6..fccfef84aa 100644 --- a/CHANGELOG.txt +++ b/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) diff --git a/CONTRIBUTORS.rst b/CONTRIBUTORS.rst index bf79c824a5..2c6222fbf1 100644 --- a/CONTRIBUTORS.rst +++ b/CONTRIBUTORS.rst @@ -501,6 +501,7 @@ Contributors * Tibor Leupold * Joan Eliot * Sagar Agarwal +* Susan Dreher Translators =========== diff --git a/docs/releases/2.13.rst b/docs/releases/2.13.rst index c6b60a5261..ca373d35d5 100644 --- a/docs/releases/2.13.rst +++ b/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 diff --git a/wagtail/core/tests/test_page_privacy.py b/wagtail/core/tests/test_page_privacy.py index 226850b9ed..5f0290caa2 100644 --- a/wagtail/core/tests/test_page_privacy.py +++ b/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/') diff --git a/wagtail/core/views.py b/wagtail/core/views.py index 2bea171fbb..63583487d2 100644 --- a/wagtail/core/views.py +++ b/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) diff --git a/wagtail/documents/tests/test_collection_privacy.py b/wagtail/documents/tests/test_collection_privacy.py index ce57d6e99f..d7edf8f5b3 100644 --- a/wagtail/documents/tests/test_collection_privacy.py +++ b/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)) diff --git a/wagtail/documents/views/serve.py b/wagtail/documents/views/serve.py index f5038b2620..de3b9606bc 100644 --- a/wagtail/documents/views/serve.py +++ b/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)