summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRadomir Dopieralski <openstack@sheep.art.pl>2020-09-07 21:03:36 +0200
committerRadomir Dopieralski <openstack@sheep.art.pl>2020-09-15 16:46:08 +0200
commit252467100f75587e18df9c43ed5802ee8f0017fa (patch)
tree8a96a4cf25a6b2315b7def15ef4d9553d0587e72
parentb6b1d70d67e3d653f09cb7d457df60f39ab2aca4 (diff)
downloadhorizon-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.py25
-rw-r--r--horizon/workflows/views.py12
-rw-r--r--releasenotes/notes/bug-cd9099c1ba78d637.yaml7
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.