From 6f98076281e9452fdb1adcd1bcbb70a6f968ade9 Mon Sep 17 00:00:00 2001
From: John Thiltges <jthiltges2@unl.edu>
Date: Wed, 2 Jan 2019 14:31:10 -0600
Subject: [PATCH] Protect against XSS vulnerabilities in URL redirection

- Switch from base64 to URL encoding for the passing the URL, using the built-in Mako filtering
- Apply HTML filtering to Mako output by default
- Disable HTML filtering for nested templates in adduser, modify, and selfmodify
---
 ldapcherry/__init__.py              | 23 ++++++++++++-----------
 resources/templates/adduser.tmpl    |  4 ++--
 resources/templates/login.tmpl      | 14 +++++++-------
 resources/templates/modify.tmpl     |  4 ++--
 resources/templates/selfmodify.tmpl |  2 +-
 5 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/ldapcherry/__init__.py b/ldapcherry/__init__.py
index 60ce654..a7d5c18 100644
--- a/ldapcherry/__init__.py
+++ b/ldapcherry/__init__.py
@@ -15,7 +15,7 @@ import logging
 import logging.handlers
 from operator import itemgetter
 from socket import error as socket_error
-import base64
+import urllib
 import cgi
 
 from exceptions import *
@@ -387,7 +387,8 @@ class LdapCherry(object):
         )
         # preload templates
         self.temp_lookup = lookup.TemplateLookup(
-            directories=self.template_dir, input_encoding='utf-8'
+            directories=self.template_dir, input_encoding='utf-8',
+            default_filters=['unicode', 'h']
             )
         # load each template
         self.temp = {}
@@ -573,7 +574,7 @@ class LdapCherry(object):
 
     def _check_auth(self, must_admin, redir_login=True):
         """ check if a user is autheticated and, optionnaly an administrator
-        if user not authentifaced -> redirection to login page (with base64
+        if user not authenticated -> redirect to login page (with escaped URL
             of the originaly requested page (redirection after login)
         if user authenticated, not admin and must_admin enabled -> 403 error
         @boolean must_admin: flag "user must be an administrator to access
@@ -588,13 +589,13 @@ class LdapCherry(object):
             qs = ''
         else:
             qs = '?' + cherrypy.request.query_string
-        # base64 of the requested URL
-        b64requrl = base64.b64encode(cherrypy.url() + qs)
+        # Escaped version of the requested URL
+        quoted_requrl = urllib.quote_plus(cherrypy.url() + qs)
         if not username:
-            # return to login page (with base64 of the url in query string
+            # return to login page (with quoted url in query string)
             if redir_login:
                 raise cherrypy.HTTPRedirect(
-                    "/signin?url=%(url)s" % {'url': b64requrl},
+                    "/signin?url=%(url)s" % {'url': quoted_requrl},
                     )
             else:
                 raise cherrypy.HTTPError(
@@ -606,7 +607,7 @@ class LdapCherry(object):
                 or not cherrypy.session['connected']:
             if redir_login:
                 raise cherrypy.HTTPRedirect(
-                    "/signin?url=%(url)s" % {'url': b64requrl},
+                    "/signin?url=%(url)s" % {'url': quoted_requrl},
                     )
             else:
                 raise cherrypy.HTTPError(
@@ -631,7 +632,7 @@ class LdapCherry(object):
         else:
             if redir_login:
                 raise cherrypy.HTTPRedirect(
-                    "/signin?url=%(url)s" % {'url': b64requrl},
+                    "/signin?url=%(url)s" % {'url': quoted_requrl},
                     )
             else:
                 raise cherrypy.HTTPError(
@@ -919,7 +920,7 @@ class LdapCherry(object):
             if url is None:
                 redirect = "/"
             else:
-                redirect = base64.b64decode(url)
+                redirect = url
             raise cherrypy.HTTPRedirect(redirect)
         else:
             message = "login failed for user '%(user)s'" % {
@@ -932,7 +933,7 @@ class LdapCherry(object):
             if url is None:
                 qs = ''
             else:
-                qs = '?url=' + url
+                qs = '?url=' + urllib.quote_plus(url)
             raise cherrypy.HTTPRedirect("/signin" + qs)
 
     @cherrypy.expose
diff --git a/resources/templates/adduser.tmpl b/resources/templates/adduser.tmpl
index 7b690fc..8b27f75 100644
--- a/resources/templates/adduser.tmpl
+++ b/resources/templates/adduser.tmpl
@@ -9,11 +9,11 @@
               <form method='POST' autocomplete="off" action='/adduser' role="form" class="form-signin" id=form>
               <fieldset>
               <legend>Fill new user's attributes:</legend>
-              ${form}
+              ${form | n}
               </fieldset>
               <fieldset>
               <legend>Enable/Disable user's roles:</legend>
-              ${roles}
+              ${roles | n}
               </fieldset>
               <div class="form-group">
                 <div class="input-group">
diff --git a/resources/templates/login.tmpl b/resources/templates/login.tmpl
index d325702..3bc6ca0 100644
--- a/resources/templates/login.tmpl
+++ b/resources/templates/login.tmpl
@@ -4,13 +4,13 @@
     <div class="row clearfix" style="margin-top:30px">
         <div class="col-md-4 column"></div>
         <div class="col-md-4 column well">
-<%
-if url is None:
-    qs=''
-else:
-    qs='?url=' + url
-%>
-            <form method='POST' action='/login${qs}' role="form" class="form-signin">
+            <form method='POST' role="form" class="form-signin"
+% if url:
+                action='login?url=${url | u}'
+% else:
+                action='login'
+% endif
+            >
               <div class="form-group">
               <h2 class="form-signin-heading">Please sign in</h2>
                 <div class="input-group">
diff --git a/resources/templates/modify.tmpl b/resources/templates/modify.tmpl
index dfeab49..b494fcc 100644
--- a/resources/templates/modify.tmpl
+++ b/resources/templates/modify.tmpl
@@ -9,11 +9,11 @@
               <form method='POST' action='/modify' role="form" class="form-signin" id="form">
               <fieldset>
               <legend>Modify user's attributes:</legend>
-              ${form}
+              ${form | n}
               </fieldset>
               <fieldset>
               <legend>Enable/Disable user's roles:</legend>
-              ${roles}
+              ${roles | n}
               </fieldset>
               % if len(standalone_groups) != 0:
               <fieldset>
diff --git a/resources/templates/selfmodify.tmpl b/resources/templates/selfmodify.tmpl
index ee58c77..95bd806 100644
--- a/resources/templates/selfmodify.tmpl
+++ b/resources/templates/selfmodify.tmpl
@@ -8,7 +8,7 @@
             <div class="well well-sm">
               <form method='POST' action='/selfmodify' autocomplete="off" role="form" class="form-signin" id="form">
               <legend>Modify your attributes:</legend>
-              ${form}
+              ${form | n}
               </fieldset>
               <div class="form-group">
                 <div class="input-group">