From 7252f2f10130bb999c2780408d5ca56325bfb95f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C4=93teris=20Caune?= <cuu508@gmail.com>
Date: Fri, 6 Aug 2021 13:34:40 +0300
Subject: [PATCH] Fix _allow_redirect function to reject absolute URLs

This fixes a security issue:
- attacker can crafts a redirect URL to an external site
- attacker gets victim to click on it
- victim logs in
- after login, Healthchecks redirects victim to the external site

The _allow_redirect function now additionally
requires the redirect URL is relative (has no scheme or domain).
---
 CHANGELOG.md                    |  1 +
 hc/accounts/tests/test_login.py | 10 ++++++++--
 hc/accounts/views.py            |  4 ++++
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 06e7bf0d..b75ead6d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@ All notable changes to this project will be documented in this file.
 ### Bug Fixes
 - Fix dark mode styling issues in Cron Syntax Cheatsheet
 - Fix a 403 when transferring a project to a read-only team member
+- Security: fix allow_redirect function to reject absolute URLs
 
 ## v1.21.0 - 2020-07-02
 
diff --git a/hc/accounts/tests/test_login.py b/hc/accounts/tests/test_login.py
index b7d3c338..fc966999 100644
--- a/hc/accounts/tests/test_login.py
+++ b/hc/accounts/tests/test_login.py
@@ -95,8 +95,14 @@ class LoginTestCase(BaseTestCase):
     def test_it_handles_bad_next_parameter(self):
         form = {"action": "login", "email": "alice@example.org", "password": "password"}
 
-        r = self.client.post("/accounts/login/?next=/evil/", form)
-        self.assertRedirects(r, self.checks_url)
+        samples = [
+            "/evil/",
+            f"https://example.org/projects/{self.project.code}/checks/",
+        ]
+
+        for sample in samples:
+            r = self.client.post("/accounts/login/?next=" + sample, form)
+            self.assertRedirects(r, self.checks_url)
 
     def test_it_handles_wrong_password(self):
         form = {
diff --git a/hc/accounts/views.py b/hc/accounts/views.py
index 8e21f8a6..e9a9b77d 100644
--- a/hc/accounts/views.py
+++ b/hc/accounts/views.py
@@ -54,6 +54,10 @@ def _allow_redirect(redirect_url):
         return False
 
     parsed = urlparse(redirect_url)
+    if parsed.netloc:
+        # Allow redirects only to relative URLs
+        return False
+
     try:
         match = resolve(parsed.path)
     except Resolver404: