[PATCH] login: strip possible prefix from came_from if it's present

[Thomas De Schampheleire]

On September 16, 2015 3:55:42 PM GMT+02:00, Andrew Shadura <andrew at shadura.me> wrote:
># HG changeset patch
># User Andrew Shadura <andrew at shadura.me>
># Date 1442411574 -7200
>#      Wed Sep 16 15:52:54 2015 +0200
># Node ID 69ea9fc01a602f290b9e78b7cd057a899fa5ff37
># Parent  889ff0f436c8b57f5962e204e699cbabc6d33aac
>login: strip possible prefix from came_from if it's present
>
>Also, reject came_from URL not belonging to our application.
>
>diff --git a/kallithea/controllers/login.py
>b/kallithea/controllers/login.py
>--- a/kallithea/controllers/login.py
>+++ b/kallithea/controllers/login.py
>@@ -63,6 +63,7 @@ class LoginController(BaseController):
> 
>         parsed = urlparse.urlparse(came_from)
>         server_parsed = urlparse.urlparse(url.current())
>+        base_prefix = request.environ.get('SCRIPT_NAME', '')
>         allowed_schemes = ['http', 'https']
>         if parsed.scheme and parsed.scheme not in allowed_schemes:
>             log.error('Suspicious URL scheme detected %s for url %s',
>@@ -72,6 +73,11 @@ class LoginController(BaseController):
>       log.error('Suspicious NETLOC detected %s for url %s server url '
>                     'is: %s' % (parsed.netloc, parsed, server_parsed))
>             return False
>+        if not parsed.path.startswith(base_prefix):
>+            log.error('Path outside of the application prefix %s for
>url %s'
>+                      'is: %s' % (base_prefix, parsed))
>+            return False
>+
>         return True
> 
>     def _redirect_to_origin(self, origin):
>@@ -81,7 +87,11 @@ class LoginController(BaseController):
> 
>     def index(self):
>         c.came_from = safe_str(request.GET.get('came_from', ''))
>-        if not self._validate_came_from(c.came_from):
>+        base_prefix = request.environ.get('SCRIPT_NAME', '')
>+
>+        if self._validate_came_from(c.came_from):
>+            c.came_from = c.came_from.split(base_prefix).pop()
>+        else:
>             c.came_from = url('home')
> 
>         not_default = self.authuser.username != User.DEFAULT_USER

I think you should add test cases here.

And regarding issue #104, did you verify that without that patch, the current 
problem was not present?

/Thomas