changeset 5473:d402d1e4aed4

security: HTTP method sanity checks This serves to document and verify some implicit constraints on the HTTP method.
author Søren Løvborg <sorenl@unity3d.com>
date Thu, 03 Sep 2015 23:49:27 +0200
parents 3598e2a4e051
children 431689d7f37d
files kallithea/lib/auth.py kallithea/tests/functional/test_admin_defaults.py
diffstat 2 files changed, 14 insertions(+), 1 deletions(-) [+]
line wrap: on
line diff
--- a/kallithea/lib/auth.py	Thu Sep 03 17:08:19 2015 +0200
+++ b/kallithea/lib/auth.py	Thu Sep 03 23:49:27 2015 +0200
@@ -760,6 +760,12 @@
                 log.warning('API access to %s is not allowed', loc)
                 return abort(403)
 
+        # Only allow the following HTTP request methods. (We sometimes use POST
+        # requests with a '_method' set to 'PUT' or 'DELETE'; but that is only
+        # used for the route lookup, and does not affect request.method.)
+        if request.method not in ['GET', 'HEAD', 'POST', 'PUT']:
+            return abort(405)
+
         # CSRF protection: Whenever a request has ambient authority (whether
         # through a session cookie or its origin IP address), it must include
         # the correct token, unless the HTTP method is GET or HEAD (and thus
@@ -772,6 +778,13 @@
                 log.error('CSRF check failed')
                 return abort(403)
 
+        # WebOb already ignores request payload parameters for anything other
+        # than POST/PUT, but double-check since other Kallithea code relies on
+        # this assumption.
+        if request.method not in ['POST', 'PUT'] and request.POST:
+            log.error('%r request with payload parameters; WebOb should have stopped this', request.method)
+            return abort(400)
+
         # regular user authentication
         if user.is_authenticated:
             log.info('user %s authenticated with regular auth @ %s', user, loc)
--- a/kallithea/tests/functional/test_admin_defaults.py	Thu Sep 03 17:08:19 2015 +0200
+++ b/kallithea/tests/functional/test_admin_defaults.py	Thu Sep 03 23:49:27 2015 +0200
@@ -64,7 +64,7 @@
 
     def test_delete(self):
         # Not possible due to CSRF protection.
-        response = self.app.delete(url('default', id=1), status=403)
+        response = self.app.delete(url('default', id=1), status=405)
 
     def test_delete_browser_fakeout(self):
         response = self.app.post(url('default', id=1), params=dict(_method='delete', _authentication_token=self.authentication_token()))