kallithea Changeset - a0a9ae753cc4

Changeset - a0a9ae753cc4

Parent rev.

Child rev.

[Not reviewed]

stable

0 2 0

Søren Løvborg - 10 years ago 2015-09-18 13:57:49
sorenl@unity3d.com

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.

2 files changed with 5 insertions and 22 deletions:

kallithea/controllers/login.py

kallithea/tests/functional/test_login.py

0 comments (0 inline, 0 general)

kallithea/controllers/login.py

➞

Show inline comments

@@ @@ -37,63 +37,50 @@ from pylons.controllers.util import redi @@
 from pylons import request, session, tmpl_context as c, url
 import kallithea.lib.helpers as h
 from kallithea.lib.auth import AuthUser, HasPermissionAnyDecorator
 from kallithea.lib.base import BaseController, log_in_user, render
 from kallithea.lib.exceptions import UserCreationError
 from kallithea.lib.utils2 import safe_str
 from kallithea.model.db import User, Setting
 from kallithea.model.forms import \
     LoginForm, RegisterForm, PasswordResetRequestForm, PasswordResetConfirmationForm
 from kallithea.model.user import UserModel
 from kallithea.model.meta import Session
 log = logging.getLogger(__name__)
 class LoginController(BaseController):
     def __before__(self):
         super(LoginController, self).__before__()
     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', ''))
         if c.came_from:
             if not self._validate_came_from(c.came_from):
                 log.error('Invalid came_from (not server-relative): %r', c.came_from)
                 raise HTTPBadRequest()
             came_from = url(c.came_from, **request.GET)
         else:
             c.came_from = came_from = url('home')
         not_default = self.authuser.username != User.DEFAULT_USER
         ip_allowed = AuthUser.check_ip_allowed(self.authuser, self.ip_addr)
         # redirect if already logged in
         if self.authuser.is_authenticated and not_default and ip_allowed:
             raise HTTPFound(location=came_from)
         if request.POST:
             # import Login Form validator class
             login_form = LoginForm()
             try:
                 c.form_result = login_form.to_python(dict(request.POST))
                 # form checks for username/password, now we're authenticated

kallithea/tests/functional/test_login.py

➞

Show inline comments

@@ @@ -84,60 +84,56 @@ class TestLoginController(TestController @@
                 'Cookie %r should have expiration date, but is a session cookie' % cookie)
     def test_logout(self):
         response = self.app.post(url(controller='login', action='index'),
                                  {'username': TEST_USER_REGULAR_LOGIN,
                                   'password': TEST_USER_REGULAR_PASS})
         # Verify that a login session has been established.
         response = self.app.get(url(controller='login', action='index'))
         response = response.follow()
         self.assertIn('authuser', response.session)
         response.click('Log Out')
         # Verify that the login session has been terminated.
         response = self.app.get(url(controller='login', action='index'))
         self.assertNotIn('authuser', response.session)
     @parameterized.expand([
           ('data:text/html,<script>window.alert("xss")</script>',),
           ('mailto:test@example.com',),
           ('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'),
                                  {'username': TEST_USER_ADMIN_LOGIN,
                                   'password': 'as'})
         self.assertEqual(response.status, '200 OK')
         response.mustcontain('Enter 3 characters or more')
     def test_login_wrong_username_password(self):
         response = self.app.post(url(controller='login', action='index'),
                                  {'username': 'error',
                                   'password': 'test12'})
         response.mustcontain('Invalid username or password')
     # verify that get arguments are correctly passed along login redirection
     @parameterized.expand([
         ({'foo':'one', 'bar':'two'}, ('foo=one', 'bar=two')),
         ({'blue': u'blå'.encode('utf-8'), 'green':u'grøn'},
              ('blue=bl%C3%A5', 'green=gr%C3%B8n')),
     ])
     def test_redirection_to_login_form_preserves_get_args(self, args, args_encoded):

0 comments (0 inline, 0 general)