diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index dd4bcc9..05e81ba 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -24,6 +24,8 @@ jobs: python-version: '3.8' - run: pip install "tinycss2>=1.2.0" - run: python noshadows.py --tests + env: + PYTHONWARNINGS: error tracdjangoplugin: runs-on: ubuntu-20.04 @@ -39,6 +41,7 @@ jobs: run: python -m django test tracdjangoplugin.tests env: DJANGO_SETTINGS_MODULE: tracdjangoplugin.settings_tests + PYTHONWARNINGS: error traccheck: runs-on: ubuntu-20.04 @@ -53,6 +56,8 @@ jobs: - run: python traccheck.py lint trac-env/ env: DJANGO_SETTINGS_MODULE: tracdjangoplugin.settings_tests + PYTHONWARNINGS: error - run: python traccheck.py components --check .TRACFREEZE.txt trac-env/ env: DJANGO_SETTINGS_MODULE: tracdjangoplugin.settings_tests + PYTHONWARNINGS: error diff --git a/DjangoPlugin/tracdjangoplugin/plugins.py b/DjangoPlugin/tracdjangoplugin/plugins.py index 4e71f56..17bdab4 100644 --- a/DjangoPlugin/tracdjangoplugin/plugins.py +++ b/DjangoPlugin/tracdjangoplugin/plugins.py @@ -11,7 +11,8 @@ from django.conf import settings from django.contrib.auth.forms import AuthenticationForm -from django.utils.http import is_safe_url +from django.utils.encoding import iri_to_uri +from django.utils.http import url_has_allowed_host_and_scheme class CustomTheme(Component): @@ -135,11 +136,14 @@ def do_post(self, req): def _get_safe_redirect_url(self, req): host = urlparse(req.base_url).hostname - redirect_url = req.args.get("next", "") or settings.LOGIN_REDIRECT_URL - if is_safe_url(redirect_url, allowed_hosts=[host]): - return redirect_url - else: - return settings.LOGIN_REDIRECT_URL + redirect_url = iri_to_uri(req.args.get("next", "")) + + if not redirect_url: + redirect_url = settings.LOGIN_REDIRECT_URL + elif not url_has_allowed_host_and_scheme(redirect_url, allowed_hosts=[host]): + redirect_url = settings.LOGIN_REDIRECT_URL + + return redirect_url class ReservedUsernamesComponent(Component): diff --git a/DjangoPlugin/tracdjangoplugin/tests.py b/DjangoPlugin/tracdjangoplugin/tests.py index 67befc3..828d089 100644 --- a/DjangoPlugin/tracdjangoplugin/tests.py +++ b/DjangoPlugin/tracdjangoplugin/tests.py @@ -85,13 +85,27 @@ def test_login_valid_with_custom_redirection_with_hostname(self): def test_login_valid_with_malicious_redirection(self): self.env.config.set("trac", "base_url", "http://localhost") User.objects.create_user(username="test", password="test") - with self.settings(LOGIN_REDIRECT_URL="/test"): - self.assertLoginSucceeds( - username="test", - password="test", - check_redirect="http://localhost/test", - extra_data={"next": "http://example.com/evil"}, - ) + + # adapted from django/tests/auth_tests/test_views.py + for redirect_url in [ + "http://example.com", + "http://example.com/evil", + "http:///example.com", + "https://example.com", + "ftp://example.com", + "///example.com", + "//example.com", + 'javascript:alert("XSS")', + ]: + with self.subTest(url=redirect_url), self.settings( + LOGIN_REDIRECT_URL="/test" + ): + self.assertLoginSucceeds( + username="test", + password="test", + check_redirect="http://localhost/test", + extra_data={"next": redirect_url}, + ) def assertLoginFails(self, username, password, error_message=None): if error_message is None: diff --git a/requirements.txt b/requirements.txt index 6125e3a..a23f914 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ # spam-filter doesn't work without babel (but somehow doesn't list it in its requirements) Trac[pygments, babel]==1.6.0 psycopg2==2.9.9 --no-binary=psycopg2 -Django==1.11.29 +Django==3.2.24 libsass==0.23.0 # Trac plugins