Mercurial > kallithea
changeset 5510:a0a9ae753cc4 stable
login: simplify came_from validation
Even though only server-relative came_from URLs were ever generated,
the login controller allowed fully qualified URLs (URLs including
scheme and server). To avoid an open HTTP redirect (CWE-601), the code
included logic to prevent redirects to other servers. By requiring
server-relative URLs, this logic can simply be removed.
Note: SCRIPT_NAME is still not validated and it is thus possible to redirect
from one app to another on the same netloc.
author | Søren Løvborg <sorenl@unity3d.com> |
---|---|
date | Fri, 18 Sep 2015 13:57:49 +0200 |
parents | ad131f703996 |
children | b537babcf966 |
files | kallithea/controllers/login.py kallithea/tests/functional/test_login.py |
diffstat | 2 files changed, 5 insertions(+), 22 deletions(-) [+] |
line wrap: on
line diff
--- a/kallithea/controllers/login.py Sun Sep 20 22:22:50 2015 +0200 +++ b/kallithea/controllers/login.py Fri Sep 18 13:57:49 2015 +0200 @@ -58,21 +58,8 @@ def _validate_came_from(self, came_from): """Return True if came_from is valid and can and should be used""" - if not came_from: - return False - - parsed = urlparse.urlparse(came_from) - server_parsed = urlparse.urlparse(url.current()) - allowed_schemes = ['http', 'https'] - if parsed.scheme and parsed.scheme not in allowed_schemes: - log.error('Suspicious URL scheme detected %s for url %s', - parsed.scheme, parsed) - return False - if server_parsed.netloc != parsed.netloc: - log.error('Suspicious NETLOC detected %s for url %s server url ' - 'is: %s' % (parsed.netloc, parsed, server_parsed)) - return False - return True + url = urlparse.urlsplit(came_from) + return not url.scheme and not url.netloc def index(self): c.came_from = safe_str(request.GET.pop('came_from', ''))
--- a/kallithea/tests/functional/test_login.py Sun Sep 20 22:22:50 2015 +0200 +++ b/kallithea/tests/functional/test_login.py Fri Sep 18 13:57:49 2015 +0200 @@ -105,18 +105,14 @@ ('file:///etc/passwd',), ('ftp://ftp.example.com',), ('http://other.example.com/bl%C3%A5b%C3%A6rgr%C3%B8d',), + ('//evil.example.com/',), ]) def test_login_bad_came_froms(self, url_came_from): response = self.app.post(url(controller='login', action='index', came_from=url_came_from), {'username': TEST_USER_ADMIN_LOGIN, - 'password': TEST_USER_ADMIN_PASS}) - self.assertEqual(response.status, '302 Found') - self.assertEqual(response._environ['paste.testing_variables'] - ['tmpl_context'].came_from, '/') - response = response.follow() - - self.assertEqual(response.status, '200 OK') + 'password': TEST_USER_ADMIN_PASS}, + status=400) def test_login_short_password(self): response = self.app.post(url(controller='login', action='index'),