[PATCH 2 of 4 prefix exploded] login: simplify came_from validation

[Mads Kiilerich]

# HG changeset patch
# User Søren Løvborg <sorenl at unity3d.com>
# Date 1442577469 -7200
#      Fri Sep 18 13:57:49 2015 +0200
# Branch stable
# Node ID 7a9470196cdc4377538fca0d389ad38719f250eb
# Parent  a61f96e3960e8752b7bf9e5812085d858369a210
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.

diff --git a/kallithea/controllers/login.py b/kallithea/controllers/login.py
--- a/kallithea/controllers/login.py
+++ b/kallithea/controllers/login.py
@@ -56,27 +56,17 @@ 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
+    def _validate_came_from(self, url):
+        """Return True if url is valid and can and should be used"""
+        url = urlparse.urlsplit(url)
+        return not url.scheme and not url.netloc
 
     def index(self):
         came_from = safe_str(request.GET.pop('came_from', ''))
-        if self._validate_came_from(came_from):
+        if came_from:
+            if not self._validate_came_from(came_from):
+                log.error('Invalid came_from (not server-relative): %r', came_from)
+                raise HTTPBadRequest()
             c.came_from = url(came_from, **request.GET)
         else:
             c.came_from = url('home')
diff --git a/kallithea/tests/functional/test_login.py b/kallithea/tests/functional/test_login.py
--- a/kallithea/tests/functional/test_login.py
+++ b/kallithea/tests/functional/test_login.py
@@ -105,18 +105,14 @@ class TestLoginController(TestController
           ('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'),