diff options
author | Radomir Dopieralski <openstack@sheep.art.pl> | 2020-09-07 21:03:36 +0200 |
---|---|---|
committer | Radomir Dopieralski <openstack@sheep.art.pl> | 2020-09-15 16:46:08 +0200 |
commit | 252467100f75587e18df9c43ed5802ee8f0017fa (patch) | |
tree | 8a96a4cf25a6b2315b7def15ef4d9553d0587e72 | |
parent | b6b1d70d67e3d653f09cb7d457df60f39ab2aca4 (diff) | |
download | horizon-252467100f75587e18df9c43ed5802ee8f0017fa.tar.gz |
Fix open redirect
Make sure the "next" URL is in the same origin as Horizon before
redirecting to it.
Change-Id: I06b2bfc8e3638591615547780c3fa34b0abe19f6
Closes-bug: #1865026
-rw-r--r-- | horizon/test/unit/workflows/test_workflows.py | 25 | ||||
-rw-r--r-- | horizon/workflows/views.py | 12 | ||||
-rw-r--r-- | releasenotes/notes/bug-cd9099c1ba78d637.yaml | 7 |
3 files changed, 42 insertions, 2 deletions
diff --git a/horizon/test/unit/workflows/test_workflows.py b/horizon/test/unit/workflows/test_workflows.py index 8ef75e686..cd872b0c9 100644 --- a/horizon/test/unit/workflows/test_workflows.py +++ b/horizon/test/unit/workflows/test_workflows.py @@ -16,6 +16,7 @@ from unittest import mock from django import forms from django import http +from django.test.utils import override_settings from horizon import base from horizon import exceptions @@ -399,3 +400,27 @@ class WorkflowsTests(test.TestCase): flow = WorkflowForTesting(req, entry_point="action_two") self.assertEqual("action_two", flow.get_entry_point()) + + @override_settings(ALLOWED_HOSTS=['localhost']) + def test_redirect_url_safe(self): + url = 'http://localhost/test' + view = WorkflowViewForTesting() + request = self.factory.get("/", data={ + 'next': url, + }) + request.META['SERVER_NAME'] = "localhost" + view.request = request + context = view.get_context_data() + self.assertEqual(url, context['REDIRECT_URL']) + + @override_settings(ALLOWED_HOSTS=['localhost']) + def test_redirect_url_unsafe(self): + url = 'http://evilcorp/test' + view = WorkflowViewForTesting() + request = self.factory.get("/", data={ + 'next': url, + }) + request.META['SERVER_NAME'] = "localhost" + view.request = request + context = view.get_context_data() + self.assertIsNone(context['REDIRECT_URL']) diff --git a/horizon/workflows/views.py b/horizon/workflows/views.py index 9c8fe1a27..107669acc 100644 --- a/horizon/workflows/views.py +++ b/horizon/workflows/views.py @@ -18,6 +18,7 @@ import json from django import forms from django import http from django import shortcuts +from django.utils import http as utils_http from django.views import generic from horizon import exceptions @@ -90,8 +91,15 @@ class WorkflowView(hz_views.ModalBackdropMixin, generic.TemplateView): workflow = self.get_workflow() workflow.verify_integrity() context[self.context_object_name] = workflow - next = self.request.GET.get(workflow.redirect_param_name) - context['REDIRECT_URL'] = next + + redirect_to = self.request.GET.get(workflow.redirect_param_name) + # Make sure the requested redirect is safe + if redirect_to and not utils_http.is_safe_url( + url=redirect_to, + allowed_hosts=[self.request.get_host()]): + redirect_to = None + context['REDIRECT_URL'] = redirect_to + context['layout'] = self.get_layout() # For consistency with Workflow class context['modal'] = 'modal' in context['layout'] diff --git a/releasenotes/notes/bug-cd9099c1ba78d637.yaml b/releasenotes/notes/bug-cd9099c1ba78d637.yaml new file mode 100644 index 000000000..438e3c30e --- /dev/null +++ b/releasenotes/notes/bug-cd9099c1ba78d637.yaml @@ -0,0 +1,7 @@ +--- +security: + - | + An open redirect has been fixed, that could redirect users to arbitrary + addresses from certain views by specifying a "next" parameter in the URL. + Now the redirect will only work if the target URL is in the same domain, + and uses the same protocol. |