From 43f04feb6c28bb3be44c9bf108db5cc746cb51e4 Mon Sep 17 00:00:00 2001 From: Chad Whitacre Date: Mon, 14 Nov 2016 10:38:01 -0800 Subject: [PATCH] Disallow POST to /assets/ --- gratipay/main.py | 2 +- gratipay/security/__init__.py | 9 +++++++-- gratipay/security/csrf.py | 3 ++- tests/py/test_security_csrf.py | 5 +++-- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/gratipay/main.py b/gratipay/main.py index eaebc89818..127fe2355c 100644 --- a/gratipay/main.py +++ b/gratipay/main.py @@ -99,6 +99,7 @@ algorithm['parse_environ_into_request'], algorithm['parse_body_into_request'], + security.only_allow_certain_methods, utils.use_tildes_for_participants, algorithm['redirect_to_base_url'], i18n.set_up_i18n, @@ -106,7 +107,6 @@ authentication.authenticate_user_if_possible, csrf.extract_token_from_cookie, csrf.reject_forgeries, - security.only_allow_certain_methods, algorithm['dispatch_request_to_filesystem'], diff --git a/gratipay/security/__init__.py b/gratipay/security/__init__.py index 7e55e87436..1f41550385 100644 --- a/gratipay/security/__init__.py +++ b/gratipay/security/__init__.py @@ -1,9 +1,14 @@ from aspen import Response +_requesting_asset = lambda r: r.path.raw.startswith('/assets/') + + def only_allow_certain_methods(request): - whitelisted = ['GET', 'HEAD', 'POST'] - if request.method.upper() not in whitelisted: + method = request.method.upper() + whitelist = ('GET', 'HEAD') if _requesting_asset(request) else ('GET', 'HEAD', 'POST') + # POSTing to /assets/ interferes with the csrf.* functions if we're not careful + if method not in whitelist: raise Response(405) diff --git a/gratipay/security/csrf.py b/gratipay/security/csrf.py index cce453a718..87f4571c3b 100644 --- a/gratipay/security/csrf.py +++ b/gratipay/security/csrf.py @@ -14,6 +14,7 @@ from aspen import Response +from . import _requesting_asset from .crypto import constant_time_compare, get_random_string @@ -37,7 +38,7 @@ def extract_token_from_cookie(request): # Don't set a CSRF cookie on assets, to avoid busting the cache. - if request.path.raw.startswith('/assets/'): + if _requesting_asset(request): token = None else: token = token or _get_new_token() diff --git a/tests/py/test_security_csrf.py b/tests/py/test_security_csrf.py index e368de4911..37b2fa3554 100644 --- a/tests/py/test_security_csrf.py +++ b/tests/py/test_security_csrf.py @@ -48,5 +48,6 @@ def test_no_csrf_cookie_set_for_assets(self): r = self.client.GET('/assets/gratipay.css') assert b'csrf_token' not in r.headers.cookie - def test_that_missing_csrf_doesnt_confuse_whatever(self): - self.client.POST('/assets/gratipay.css') + def test_that_missing_csrf_on_assets_doesnt_result_in_a_500(self): + r = self.client.PxST('/assets/gratipay.css') + assert r.code == 405