Merged

Author: schlatterbeck

.travis.yml

@@ -65,7 +65,8 @@ before_install:
   - cd $TRAVIS_BUILD_DIR
 
 install:
-  - pip install gpg mysqlclient psycopg2 pytz whoosh
+  - pip install mysqlclient==1.3.13
+  - pip install gpg psycopg2 pytz whoosh
   - pip install pytest-cov codecov
 
 before_script:

CHANGES.txt

@@ -100,6 +100,10 @@ Fixed:
   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, reviewed/applied John Rouillard.)
+- issue2551035 - fix XSS issue in wsgi and cgi when handing url not
+  found/404. Reported by hannob at
+  https://github.com/python/bugs.python.org/issues/34, issue opened by
+  JulienPalard.
 
 2018-07-13 1.6.0
 

frontends/roundup.cgi

@@ -181,7 +181,7 @@ def main(out, err):
                 request.send_response(404)
                 request.send_header('Content-Type', 'text/html')
                 request.end_headers()
-                out.write(s2b('Not found: %s'%client.path))
+                out.write(s2b('Not found: %s'%cgi.escape(client.path)))
 
     else:
         from roundup.anypy import urllib_

roundup/cgi/client.py

@@ -237,15 +237,13 @@ class BinaryFieldStorage(cgi.FieldStorage):
        needed for handling json and xml data blobs under python
        3. Under python 2, str and binary are interchangable, not so
        under 3.
-
-       Note that there may be places where this should support text mode.
-       (e.g. a large text file upload??). None are known, but this could be
-       a problem.
     '''
     def make_file(self, mode=None):
         ''' work around https://bugs.python.org/issue27777 '''
         import tempfile
-        return tempfile.TemporaryFile("wb+")
+        if self.length >= 0:
+            return tempfile.TemporaryFile("wb+")
+        return super().make_file()
 
 class Client:
     """Instantiate to handle one CGI request.
@@ -527,7 +525,16 @@ def handle_rest(self):
         self.determine_language()
         # Open the database as the correct user.
         # TODO: add everything to RestfulDispatcher
-        self.determine_user()
+        try:
+            self.determine_user()
+        except LoginError as err:
+            self.response_code = http_.client.UNAUTHORIZED
+            output = b"Invalid Login\n"
+            self.setHeader("Content-Length", str(len(output)))
+            self.setHeader("Content-Type", "text/plain")
+            self.write(output)
+            return
+
         self.check_anonymous_access()
 
         # Call rest library to handle the request

roundup/cgi/wsgi_handler.py

@@ -69,7 +69,7 @@ def __call__(self, environ, start_response):
             client.main()
         except roundup.cgi.client.NotFound:
             request.start_response([('Content-Type', 'text/html')], 404)
-            request.wfile.write(s2b('Not found: %s'%client.path))
+            request.wfile.write(s2b('Not found: %s'%cgi.escape(client.path)))
 
         # all body data has been written using wfile
         return []

roundup/rest.py

@@ -132,7 +132,7 @@ def calculate_etag (node, classname="Missing", id="0"):
     '''
 
     items = node.items(protected=True) # include every item
-    etag = md5(bs2b(repr(items))).hexdigest()
+    etag = md5(bs2b(repr(sorted(items)))).hexdigest()
     logger.debug("object=%s%s; tag=%s; repr=%s", classname, id,
                  etag, repr(node.items(protected=True)))
     return etag
@@ -1336,7 +1336,7 @@ def summary(self, input):
             summary.setdefault(status_name, []).append(issue_object)
             messages.append((num, issue_object))
 
-        messages.sort(reverse=True)
+        sorted(messages, key=lambda tup: tup[0], reverse=True)
 
         result = {
             'created': created,
@@ -1450,7 +1450,7 @@ def dispatch(self, method, uri, input):
             output = RoundupJSONEncoder(indent=indent).encode(output)
         elif data_type.lower() == "xml" and dicttoxml:
             self.client.setHeader("Content-Type", "application/xml")
-            output = dicttoxml(output, root=False)
+            output = b2s(dicttoxml(output, root=False))
         else:
             self.client.response_code = 406
             output = "Content type is not accepted by client"

roundup/scripts/roundup_server.py

@@ -377,13 +377,15 @@ def inner_run_cgi(self):
             env['QUERY_STRING'] = query
         if hasattr(self.headers, 'get_content_type'):
             # Python 3.  We need the raw header contents.
-            env['CONTENT_TYPE'] = self.headers.get('content-type')
+            content_type = self.headers.get('content-type')
         elif self.headers.typeheader is None:
             # Python 2.
-            env['CONTENT_TYPE'] = self.headers.type
+            content_type = self.headers.type
         else:
             # Python 2.
-            env['CONTENT_TYPE'] = self.headers.typeheader
+             content_type = self.headers.typeheader
+        if content_type:
+            env['CONTENT_TYPE'] = content_type
         length = self.headers.get('content-length')
         if length:
             env['CONTENT_LENGTH'] = length

test/rest_common.py

@@ -346,6 +346,32 @@ def testPagination(self):
         #   page_size < 0
         #   page_index < 0
 
+    def notestEtagGeneration(self):
+        ''' Make sure etag generation is stable
+        
+            FIXME need to mock somehow date.Date() when creating
+            the target to be mocked. The differening dates makes
+            this test impossible.
+        '''
+        newuser = self.db.user.create(
+            username='john',
+            password=password.Password('random1'),
+            address='[email protected]',
+            realname='JohnRandom',
+            roles='User,Admin'
+        )
+
+        node = self.db.user.getnode(self.joeid)
+        etag = calculate_etag(node)
+        items = node.items(protected=True) # include every item
+        print(repr(items))
+        print(etag)
+        self.assertEqual(etag, "6adf97f83acf6453d4a6a4b1070f3754")
+
+        etag = calculate_etag(self.db.issue.getnode("1"))
+        print(etag)
+        self.assertEqual(etag, "6adf97f83acf6453d4a6a4b1070f3754")
+        
     def testEtagProcessing(self):
         '''
         Etags can come from two places: