Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-c97h-m5qf-j8mf
Fix open redirect vulnerability in cookie list page
  • Loading branch information
sergei-maertens committed Jun 13, 2022
2 parents 908a36a + d203685 commit 3e8c9cc
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 3 deletions.
18 changes: 18 additions & 0 deletions src/openforms/forms/templatetags/openforms.py
Expand Up @@ -4,6 +4,7 @@
from rest_framework.reverse import reverse

from openforms.config.models import GlobalConfiguration
from openforms.utils.redirect import allow_redirect_url

from ..context_processors import sdk_urls

Expand All @@ -30,3 +31,20 @@ def sdk_info_banner():
"enabled": config.display_sdk_information,
**sdk_urls(request=None),
}


@register.simple_tag
def get_allowed_redirect_url(*candidates: str) -> str:
"""
Output the first variable passed that is not empty and is an allowed redirect URL.
Output nothing if none of the values satisfy the requirements.
Heavily insired on the builtin {% firstof %} tag.
"""
for candidate in candidates:
if not candidate:
continue
if allow_redirect_url(candidate):
return candidate
return ""
4 changes: 2 additions & 2 deletions src/openforms/templates/cookie_consent/cookiegroup_list.html
@@ -1,9 +1,9 @@
{% extends 'ui/views/abstract/list.html' %}
{% load i18n %}
{% load i18n openforms %}


{% block card %}
{% firstof request.GET.referer request.headers.referer as referer %}
{% get_allowed_redirect_url request.GET.referer request.headers.referer as referer %}
<header class="card__header">
<h1 class="title">{% trans "Cookies" %}</h1>
</header>
Expand Down
47 changes: 46 additions & 1 deletion src/openforms/tests/test_cookie_notice.py
@@ -1,12 +1,14 @@
from io import StringIO

from django.core.management import call_command
from django.test import override_settings
from django.test import override_settings, tag
from django.urls import reverse
from django.utils.translation import gettext as _

from cookie_consent.cache import delete_cache
from cookie_consent.models import CookieGroup
from django_webtest import WebTest
from furl import furl

from openforms.config.models import GlobalConfiguration
from openforms.forms.tests.factories import FormFactory
Expand Down Expand Up @@ -122,3 +124,46 @@ def test_analytics_snippets_not_rendered(self):
self.assertTemplateUsed(
refreshed_form_page, "includes/analytics/all_bottom.html"
)

@tag("GHSA-c97h-m5qf-j8mf")
@override_settings(
CORS_ALLOW_ALL_ORIGINS=False,
CORS_ALLOWED_ORIGINS=["https://external.domain.com"],
ALLOWED_HOSTS=["testserver", "example.com"],
IS_HTTPS=True,
)
def test_accept_reject_does_not_allow_open_redirect(self):
url = reverse("cookie_consent_cookie_group_list")
allowed_redirects = (
"https://example.com/foo/bar",
"https://testserver/admin/",
"/admin/",
)
blocked_redirects = (
"http://example.com",
"https://evil.com",
)

for allowed in allowed_redirects:
with self.subTest(f"Allowed redirect to '{allowed}'"):
self.renew_app()

cookies_page = self.app.get(url, {"referer": allowed})

button = cookies_page.pyquery.find("a.button--primary")
self.assertEqual(button.attr["href"], allowed)
self.assertEqual(button.text(), _("Close"))

for blocked in blocked_redirects:
with self.subTest(f"Blockedredirect to '{blocked}'"):
self.renew_app()

cookies_page = self.app.get(url, {"referer": blocked})

button = cookies_page.pyquery.find("a.button--primary")
self.assertFalse(button)

for form in cookies_page.forms.values():
next_url = furl(form["next"].value)

self.assertEqual(next_url.args.get("referer", ""), "")
17 changes: 17 additions & 0 deletions src/openforms/utils/redirect.py
@@ -1,5 +1,8 @@
from urllib.parse import urlparse, urlunparse

from django.conf import settings
from django.utils.http import url_has_allowed_host_and_scheme

from corsheaders.conf import conf as cors_conf
from corsheaders.middleware import CorsMiddleware

Expand All @@ -18,6 +21,20 @@ def allow_redirect_url(url: str) -> bool:
are allowed to access Open Forms. We leverage this configuration to block or allow
redirects to external hosts.
"""
# first, check if the URL is in ALLOWED_HOSTS. We deliberately exclude the wildcard
# setting to require explicit configuration either via ALLOWED_HOSTS or CORS_* settings.
allowed_hosts_check = url_has_allowed_host_and_scheme(
url=url,
allowed_hosts=[host for host in settings.ALLOWED_HOSTS if host != "*"],
# settings.ALLOWED_HOSTS means we are serving the domain, so we can enforce our
# own custom HTTPS setting.
require_https=settings.IS_HTTPS,
)
# if we pass via ALLOWED_HOSTS, short-circuit, otherwise we check the CORS policy
# for allowed external domains.
if allowed_hosts_check:
return True

cors = CorsMiddleware()
origin = origin_from_url(url)
parts = urlparse(url)
Expand Down

0 comments on commit 3e8c9cc

Please sign in to comment.