Mercurial > p > roundup > code
diff frontends/roundup.cgi @ 8558:5fbf6451a782
bug: harden header/environment values for roundup-server and cgi
If the environment (cgi) or header variables (server) have values with
characters outside of the printable ascii range (chr(32-126)), return
HTTP 400 error. This is overly strict but nothing that Roundup looks
at requires a larger range.
When deploying with wsgi and Zope, server software should verify
proper values.
This fix was riggered by the waitress wsgi server bug:
https://github.com/Pylons/waitress/security/advisories/GHSA-m5ff-3wj3-8ph4
which was caused by incorrect validation of header values resulting in
a the proxy and waitress having different interpretations of what the
header meant.
My testing of the roundup.cgi script is to use a cgi->wsgi wrapper and
run it under wsgi (using waitress). I need to try it under a real
server that can run cgi. It looks like python http.server --cgi is
missing definitions of HTTP_HOST and other required CGI
variables. That's probably why the --cgi option was removed, but it
leaves me without a good way to test.
Maybe https://github.com/mdklatt/pytest-cgi could be used to test that
front end? Arguably CGI is old, but cheap hosting still allows it.
| author | John Rouillard <rouilj@ieee.org> |
|---|---|
| date | Wed, 08 Apr 2026 00:35:34 -0400 |
| parents | 978285986b2c |
| children | 646ba821f63e |
line wrap: on
line diff
--- a/frontends/roundup.cgi Mon Apr 06 22:10:23 2026 -0400 +++ b/frontends/roundup.cgi Wed Apr 08 00:35:34 2026 -0400 @@ -18,11 +18,14 @@ # python version check from __future__ import print_function + +import sys +import time + from roundup import version_check -from roundup.i18n import _ from roundup.anypy.html import html_escape -from roundup.anypy.strings import s2b, StringIO -import sys, time +from roundup.anypy.strings import StringIO, s2b +from roundup.i18n import _ # ## Configuration @@ -31,17 +34,17 @@ # Configuration can also be provided through the OS environment (or via # the Apache "SetEnv" configuration directive). If the variables # documented below are set, they _override_ any configuation defaults -# given in this file. +# given in this file. # TRACKER_HOMES is a list of trackers, in the form # "NAME=DIR<sep>NAME2=DIR2<sep>...", where <sep> is the directory path -# separator (";" on Windows, ":" on Unix). +# separator (";" on Windows, ":" on Unix). -# Make sure the NAME part doesn't include any url-unsafe characters like +# Make sure the NAME part doesn't include any url-unsafe characters like # spaces, as these confuse the cookie handling in browsers like IE. # ROUNDUP_LOG is the name of the logfile; if it's empty or does not exist, -# logging is turned off (unless you changed the default below). +# logging is turned off (unless you changed the default below). # DEBUG_TO_CLIENT specifies whether debugging goes to the HTTP server (via # stderr) or to the web client (via cgitb). @@ -52,15 +55,20 @@ # 'example': '/path/to/example', } + # Where to log debugging information to. Use an instance of DevNull if you # don't want to log anywhere. class DevNull: def write(self, info): pass + def close(self): pass + def flush(self): pass + + #LOG = open('/var/log/roundup.cgi.log', 'a') LOG = DevNull() @@ -71,11 +79,12 @@ # # Set up the error handler -# +# try: import traceback + from roundup.cgi import cgitb -except: +except ImportError: print("Content-Type: text/plain\n") print(_("Failed to import cgitb!\n\n")) s = StringIO() @@ -100,13 +109,13 @@ TRACKER_HOMES = {} for home in homes.split(os.pathsep): try: - name, dir = home.split('=', 1) + name, dir_ = home.split('=', 1) except ValueError: # ignore invalid definitions continue - if name and dir: - TRACKER_HOMES[name] = dir - + if name and dir_: + TRACKER_HOMES[name] = dir_ + logname = os.environ.get('ROUNDUP_LOG', '') if logname: LOG = open(logname, 'a') @@ -123,28 +132,62 @@ def __init__(self, wfile): self.rfile = sys.stdin self.wfile = wfile + def write(self, data): self.wfile.write(data) + def send_response(self, code): - self.write(s2b('Status: %s\r\n'%code)) + self.write(s2b('Status: %s\r\n' % code)) + def send_header(self, keyword, value): self.write(s2b("%s: %s\r\n" % (keyword, value))) + def end_headers(self): self.write(b"\r\n") + def start_response(self, headers, response): self.send_response(response) for key, value in headers: self.send_header(key, value) self.end_headers() + # # Main CGI handler # def main(out, err): import os + + import roundup.cgi.client import roundup.instance + + request = RequestWrapper(out) + problem_header = roundup.cgi.client.are_header_values_safe(os.environ) + if problem_header: + error_body = """<!DOCTYPE HTML> + <html lang="en"> + <head> + <meta charset="utf-8"> + <title>Error response</title> + </head> + <body> + <h1>Error response</h1> + <p>Error code: 400</p> + <p>Message: Invalid value: %s.</p> + <p>Error code explanation: 400 - The value for the header is unacceptable.</p> + </body> + </html> + """ % problem_header + + request.send_response(400) + request.send_header('Content-Type', 'text/html') + request.send_header('Content-Length', '%s' % len(error_body)) + request.end_headers() + out.write(s2b(error_body)) + return + + path = os.environ.get('PATH_INFO', '/').split('/') - request = RequestWrapper(out) request.path = os.environ.get('PATH_INFO', '/') tracker = path[1] os.environ['TRACKER_NAME'] = tracker @@ -158,7 +201,7 @@ protocol = 'https' else: protocol = 'http' - absolute_url = '%s://%s%s/'%(protocol, os.environ['HTTP_HOST'], + absolute_url = '%s://%s%s/' % (protocol, os.environ['HTTP_HOST'], os.environ.get('REQUEST_URI', '')) request.send_header('Location', absolute_url) request.end_headers() @@ -166,7 +209,6 @@ else: tracker_home = TRACKER_HOMES[tracker] tracker = roundup.instance.open(tracker_home) - import roundup.cgi.client if hasattr(tracker, 'Client'): client = tracker.Client(tracker, request, os.environ) else: @@ -182,7 +224,7 @@ request.send_response(404) request.send_header('Content-Type', 'text/html') request.end_headers() - out.write(s2b('Not found: %s'%html_escape(client.path))) + out.write(s2b('Not found: %s' % html_escape(client.path))) else: from roundup.anypy import urllib_ @@ -194,12 +236,13 @@ w(s2b(_('<body><h1>Roundup trackers index</h1><ol>\n'))) homes = sorted(TRACKER_HOMES.keys()) for tracker in homes: - w(s2b(_('<li><a href="%(tracker_url)s/index">%(tracker_name)s</a>\n')%{ - 'tracker_url': os.environ['SCRIPT_NAME']+'/'+ + w(s2b(_('<li><a href="%(tracker_url)s/index">%(tracker_name)s</a>\n') % { + 'tracker_url': os.environ['SCRIPT_NAME'] + '/' + urllib_.quote(tracker), 'tracker_name': html_escape(tracker)})) w(s2b(_('</ol></body></html>'))) + # # Now do the actual CGI handling # @@ -207,7 +250,8 @@ try: # force input/output to binary (important for file up/downloads) if sys.platform == "win32": - import os, msvcrt + import msvcrt + import os msvcrt.setmode(sys.stdin.fileno(), os.O_BINARY) msvcrt.setmode(sys.stdout.fileno(), os.O_BINARY) checkconfig() @@ -228,7 +272,7 @@ out.write(cgitb.breaker()) ts = time.ctime() out.write('''<p>%s: An error occurred. Please check - the server log for more information.</p>'''%ts) + the server log for more information.</p>''' % ts) print('EXCEPTION AT', ts, file=sys.stderr) traceback.print_exc(0, sys.stderr)