diff options
author | Zuul <zuul@review.opendev.org> | 2020-12-27 18:02:40 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2020-12-27 18:02:40 +0000 |
commit | 37af3f3d0a7f2d50c759964e3a46f4beeee27f15 (patch) | |
tree | aeb5491d2a8dc93f7d4b754c0c57ab62a89480c9 | |
parent | 7da8c27ae5164e1c5ef0dd7cc966a2f8985b4681 (diff) | |
parent | cd4e6db77a62b5f75a6d3d291d95560ae78b0827 (diff) | |
download | horizon-stable/pike.tar.gz |
Merge "Fix open redirect" into stable/pikepike-eolstable/pike
-rw-r--r-- | horizon/test/tests/workflows.py | 26 | ||||
-rw-r--r-- | horizon/workflows/views.py | 12 | ||||
-rw-r--r-- | releasenotes/notes/bug-cd9099c1ba78d637.yaml | 7 |
3 files changed, 42 insertions, 3 deletions
diff --git a/horizon/test/tests/workflows.py b/horizon/test/tests/workflows.py index fd9350e3b..5a336ec8e 100644 --- a/horizon/test/tests/workflows.py +++ b/horizon/test/tests/workflows.py @@ -14,8 +14,8 @@ from django import forms from django import http +from django.test.utils import override_settings import mock - import six from horizon import exceptions @@ -359,3 +359,27 @@ class WorkflowsTests(test.TestCase): flow = TestWorkflow(req, entry_point="test_action_two") self.assertEqual("test_action_two", flow.get_entry_point()) + + @override_settings(ALLOWED_HOSTS=['localhost']) + def test_redirect_url_safe(self): + url = 'http://localhost/test' + view = TestWorkflowView() + 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 = TestWorkflowView() + 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 0ce15a8f1..94bdbc35a 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 import six @@ -92,8 +93,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, + host=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. |