Skip to content

Commit 02fae16

Browse files
committed
issue2551023: Fix CSRF headers for use with wsgi and cgi. The
env variable array used - separators rather than _. Compare: HTTP_X-REQUESTED-WITH to HTTP_X_REQUESTED_WITH. The last is correct. Also fix roundup-server to produce the latter form. (Patch by Cédric Krier)
1 parent 23f821a commit 02fae16

File tree

4 files changed

+19
-15
lines changed

4 files changed

+19
-15
lines changed

CHANGES.txt

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,11 @@ Fixed:
8282
headers. (Joseph Myers)
8383
- issue2551022: support non-ASCII prefixes in instance config for
8484
finding static files. (C�dric Krier)
85-
85+
- issue2551023: Fix CSRF headers for use with wsgi and cgi. The
86+
env variable array used - separators rather than _. Compare:
87+
HTTP_X-REQUESTED-WITH to HTTP_X_REQUESTED_WITH. The last is
88+
correct. Also fix roundup-server to produce the latter form. (Patch
89+
by C�dric Krier, reviewed/applied John Rouillard.)
8690

8791
2018-07-13 1.6.0
8892

roundup/cgi/client.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,7 +1074,7 @@ def handle_csrf(self, xmlrpc=False):
10741074
# If required headers are missing, raise an error
10751075
for header in header_names:
10761076
if (config["WEB_CSRF_ENFORCE_HEADER_%s"%header] == 'required'
1077-
and "HTTP_%s"%header not in self.env):
1077+
and "HTTP_%s" % header.replace('-', '_') not in self.env):
10781078
logger.error(self._("csrf header %s required but missing for user%s."), header, current_user)
10791079
raise Unauthorised(self._("Missing header: %s")%header)
10801080

@@ -1110,9 +1110,9 @@ def handle_csrf(self, xmlrpc=False):
11101110
header_pass += 1
11111111

11121112
enforce=config['WEB_CSRF_ENFORCE_HEADER_X-FORWARDED-HOST']
1113-
if 'HTTP_X-FORWARDED-HOST' in self.env:
1113+
if 'HTTP_X_FORWARDED_HOST' in self.env:
11141114
if enforce != "no":
1115-
host = self.env['HTTP_X-FORWARDED-HOST']
1115+
host = self.env['HTTP_X_FORWARDED_HOST']
11161116
foundat = self.base.find('://' + host + '/')
11171117
# 4 means self.base has http:/ prefix, 5 means https:/ prefix
11181118
if foundat not in [4, 5]:
@@ -1159,7 +1159,7 @@ def handle_csrf(self, xmlrpc=False):
11591159
# Note we do not use CSRF nonces for xmlrpc requests.
11601160
#
11611161
# see: https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet#Protecting_REST_Services:_Use_of_Custom_Request_Headers
1162-
if 'HTTP_X-REQUESTED-WITH' not in self.env:
1162+
if 'HTTP_X_REQUESTED_WITH' not in self.env:
11631163
logger.error(self._("csrf X-REQUESTED-WITH xmlrpc required header check failed for user%s."), current_user)
11641164
raise UsageError(self._("Required Header Missing"))
11651165

roundup/scripts/roundup_server.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,7 @@ def inner_run_cgi(self):
411411
# If behind a proxy, this is the hostname supplied
412412
# via the Host header to the proxy. Used by core code.
413413
# Controlled by the CSRF settings.
414-
env['HTTP_X-FORWARDED-HOST'] = xfh
414+
env['HTTP_X_FORWARDED_HOST'] = xfh
415415
xff = self.headers.get('X-Forwarded-For', None)
416416
if xff:
417417
# xff is a list of ip addresses for original client/proxies:
@@ -421,7 +421,7 @@ def inner_run_cgi(self):
421421
# Made available for extensions if the user trusts it.
422422
# E.g. you may wish to disable recaptcha validation extension
423423
# if the ip of the client matches 172.16.0.0.
424-
env['HTTP_X-FORWARDED-FOR'] = xff
424+
env['HTTP_X_FORWARDED_FOR'] = xff
425425
xfp = self.headers.get('X-Forwarded-Proto', None)
426426
if xfp:
427427
# xfp is the protocol (http/https) seen by proxies in the
@@ -435,7 +435,7 @@ def inner_run_cgi(self):
435435
# May not be trustworthy. Do not use in core without
436436
# config option to control its use.
437437
# Made available for extensions if the user trusts it.
438-
env['HTTP_X-FORWARDED-PROTO'] = xfp
438+
env['HTTP_X_FORWARDED_PROTO'] = xfp
439439
if 'CGI_SHOW_TIMING' in os.environ:
440440
env['CGI_SHOW_TIMING'] = os.environ['CGI_SHOW_TIMING']
441441
env['HTTP_ACCEPT_LANGUAGE'] = self.headers.get('accept-language')
@@ -447,7 +447,7 @@ def inner_run_cgi(self):
447447
env['HTTP_ORIGIN'] = origin
448448
xrw = self.headers.get('x-requested-with')
449449
if xrw:
450-
env['HTTP_X-REQUESTED-WITH'] = xrw
450+
env['HTTP_X_REQUESTED_WITH'] = xrw
451451
range = self.headers.get('range')
452452
if range:
453453
env['HTTP_RANGE'] = range

test/test_cgi.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -904,7 +904,7 @@ def hasPermission(s, p, classname=None, d=None, e=None, **kw):
904904
del(cl.env['HTTP_ORIGIN'])
905905
del(out[0])
906906

907-
cl.env['HTTP_X-FORWARDED-HOST'] = 'whoami.com'
907+
cl.env['HTTP_X_FORWARDED_HOST'] = 'whoami.com'
908908
# if there is an X-FORWARDED-HOST header it is used and
909909
# HOST header is ignored. X-FORWARDED-HOST should only be
910910
# passed/set by a proxy. In this case the HOST header is
@@ -915,7 +915,7 @@ def hasPermission(s, p, classname=None, d=None, e=None, **kw):
915915
match_at=out[0].find('Redirecting to <a href="http://whoami.com/path/issue1?@ok_message')
916916
print("result of subtest 4:", out[0])
917917
self.assertNotEqual(match_at, -1)
918-
del(cl.env['HTTP_X-FORWARDED-HOST'])
918+
del(cl.env['HTTP_X_FORWARDED_HOST'])
919919
del(cl.env['HTTP_HOST'])
920920
del(out[0])
921921

@@ -928,14 +928,14 @@ def hasPermission(s, p, classname=None, d=None, e=None, **kw):
928928
del(out[0])
929929

930930
# try failing headers
931-
cl.env['HTTP_X-FORWARDED-HOST'] = 'whoami.net'
931+
cl.env['HTTP_X_FORWARDED_HOST'] = 'whoami.net'
932932
# this raises an error as the header check passes and
933933
# it did the edit and tries to send mail.
934934
cl.inner_main()
935935
match_at=out[0].find('Invalid X-FORWARDED-HOST whoami.net')
936936
print("result of subtest 6:", out[0])
937937
self.assertNotEqual(match_at, -1)
938-
del(cl.env['HTTP_X-FORWARDED-HOST'])
938+
del(cl.env['HTTP_X_FORWARDED_HOST'])
939939
del(out[0])
940940

941941
# header checks succeed
@@ -1047,7 +1047,7 @@ def wh(s):
10471047
'CONTENT_TYPE': 'text/plain',
10481048
'HTTP_AUTHORIZATION': 'Basic YWRtaW46YWRtaW4=',
10491049
'HTTP_REFERER': 'http://whoami.com/path/',
1050-
'HTTP_X-REQUESTED-WITH': "XMLHttpRequest"
1050+
'HTTP_X_REQUESTED_WITH': "XMLHttpRequest"
10511051
}, form)
10521052
cl.db = self.db
10531053
cl.base = 'http://whoami.com/path/'
@@ -1075,7 +1075,7 @@ def wh(s):
10751075
del(out[0])
10761076

10771077
# remove the X-REQUESTED-WITH header and get an xmlrpc fault returned
1078-
del(cl.env['HTTP_X-REQUESTED-WITH'])
1078+
del(cl.env['HTTP_X_REQUESTED_WITH'])
10791079
cl.handle_xmlrpc()
10801080
frag_faultCode = "<member>\n<name>faultCode</name>\n<value><int>1</int></value>\n</member>\n"
10811081
frag_faultString = "<member>\n<name>faultString</name>\n<value><string>&lt;class 'roundup.exceptions.UsageError'&gt;:Required Header Missing</string></value>\n</member>\n"

0 commit comments

Comments
 (0)