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).
This commit is contained in:
Pēteris Caune 2021-08-06 13:34:40 +03:00
parent f85aec225d
commit 7252f2f101
No known key found for this signature in database
GPG Key ID: E28D7679E9A9EDE2
3 changed files with 13 additions and 2 deletions

View File

@ -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

View File

@ -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 = {

View File

@ -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: