# HG changeset patch
# User Søren Løvborg <sorenl@unity3d.com>
# Date 2015-09-03 23:49:27
# Node ID d402d1e4aed43b3568f3af8a0b98451c1edf14df
# Parent  3598e2a4e0511b71eba9a23f74b91f923c923a45

security: HTTP method sanity checks

This serves to document and verify some implicit constraints on the
HTTP method.

diff --git a/kallithea/lib/auth.py b/kallithea/lib/auth.py
--- a/kallithea/lib/auth.py
+++ b/kallithea/lib/auth.py
@@ -760,6 +760,12 @@ class LoginRequired(object):
                 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 @@ class LoginRequired(object):
                 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)
diff --git a/kallithea/tests/functional/test_admin_defaults.py b/kallithea/tests/functional/test_admin_defaults.py
--- a/kallithea/tests/functional/test_admin_defaults.py
+++ b/kallithea/tests/functional/test_admin_defaults.py
@@ -64,7 +64,7 @@ class TestDefaultsController(TestControl
 
     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()))