# HG changeset patch # User John Rouillard # Date 1735517461 18000 # Node ID 57325fea99826845b4b8f2e8baaaf4eaeea59e48 # Parent 2d0bd038fc5e0c9271bf7f9664f783b21e958c76 issue2551116 - Replace xmlrpclib (xmlrpc.client) with defusedxml. defusedxml will be used to moneypatch the problematic client and server modules. Test added using an xml bomb. diff -r 2d0bd038fc5e -r 57325fea9982 CHANGES.txt --- a/CHANGES.txt Mon Dec 23 21:10:54 2024 -0500 +++ b/CHANGES.txt Sun Dec 29 19:11:01 2024 -0500 @@ -81,6 +81,9 @@ Rouillard) - added fuzz testing for some code. Found issue2551382 and others. (John Rouillard) +- issue2551116 - Replace xmlrpclib (xmlrpc.client) with defusedxml. + Added support for defusedxml to better secure the xmlrpc + endpoint. (John Rouillard) 2024-07-13 2.4.0 diff -r 2d0bd038fc5e -r 57325fea9982 doc/installation.txt --- a/doc/installation.txt Mon Dec 23 21:10:54 2024 -0500 +++ b/doc/installation.txt Sun Dec 29 19:11:01 2024 -0500 @@ -255,6 +255,11 @@ its TEMPLATE-INFO.txt file) you need to have the jinja2_ template engine installed. +defusedxml + If you are going to enable and use the XMLRPC endpoint, you should + install the defusedxml_ module. It will still work with the default + xmlrpc standard library, but it will log a warning when used. + .. _install/docutils: docutils @@ -2371,6 +2376,7 @@ .. _apache: https://httpd.apache.org/ .. _brotli: https://pypi.org/project/Brotli/ .. _`developer's guide`: developers.html +.. _defusedxml: https://pypi.org/project/defusedxml/ .. _docutils: https://pypi.org/project/docutils/ .. _flup: https://pypi.org/project/flup/ .. _gpg: https://www.gnupg.org/software/gpgme/index.html diff -r 2d0bd038fc5e -r 57325fea9982 doc/upgrading.txt --- a/doc/upgrading.txt Mon Dec 23 21:10:54 2024 -0500 +++ b/doc/upgrading.txt Sun Dec 29 19:11:01 2024 -0500 @@ -159,6 +159,36 @@ add the lines marked with ``+`` in the file in the location after check_main is assigned. +Defusedxml support improves XMLRPC security (optional) +------------------------------------------------------ + +This release adds support for the defusedxml_ module. If it is +installed it will be automatically used. The default xmlrpc module in +the standard library has known issues when parsing crafted XML. It can +take a lot of CPU time and consume large amounts of memory with small +payloads. + +When the XMLRPC endpoint is used without defusedxml, it will log a +warning to the log file. The log entry can be disabled by adding:: + + + from roundup.cgi import client + client.WARN_FOR_MISSING_DEFUSEDXML = False + +to the ``interfaces.py`` file in the tracker home. (Create the file if +it is missing.) + +XMLRPC access is enabled by default in the classic and other trackers. +Upgrading to defusedxml is considered optional because the XMLRPC +endpoint can be disabled in the tracker's ``config.ini``. Also +``Xmlrpc Access`` can be removed from the ``Users`` role by commenting +out a line in ``schema.py``. + +If you have enabled the xmlrpc endpoint, you should install +defusedxml. + +.. _defusedxml: https://pypi.org/project/defusedxml/ + More secure session cookie handling (info) ------------------------------------------ diff -r 2d0bd038fc5e -r 57325fea9982 doc/xmlrpc.txt --- a/doc/xmlrpc.txt Mon Dec 23 21:10:54 2024 -0500 +++ b/doc/xmlrpc.txt Sun Dec 29 19:11:01 2024 -0500 @@ -79,7 +79,8 @@ default python XML parser. This parser is know to have security issues. For details see: https://pypi.org/project/defusedxml/. You may wish to use the rest interface which doesn't have the same -issues. Patches with tests to roundup to use defusedxml are welcome. +issues. If you install defusedxml, it will be automatically used to add +some additional protection. .. caution:: diff -r 2d0bd038fc5e -r 57325fea9982 roundup/anypy/xmlrpc_.py --- a/roundup/anypy/xmlrpc_.py Mon Dec 23 21:10:54 2024 -0500 +++ b/roundup/anypy/xmlrpc_.py Sun Dec 29 19:11:01 2024 -0500 @@ -1,6 +1,18 @@ try: # Python 3+. from xmlrpc import client, server + # If client.defusedxml == False, client.py will warn that + # xmlrpc is insecure and defusedxml should be installed. + client.defusedxml=False + try: + from defusedxml import xmlrpc + xmlrpc.monkey_patch() + # figure out how to allow user to set xmlrpc.MAX_DATA = bytes + client.defusedxml=True + except ImportError: + # use regular xmlrpc with warnings + pass + server.SimpleXMLRPCDispatcher except (ImportError, AttributeError): # Python 2. diff -r 2d0bd038fc5e -r 57325fea9982 roundup/cgi/client.py --- a/roundup/cgi/client.py Mon Dec 23 21:10:54 2024 -0500 +++ b/roundup/cgi/client.py Sun Dec 29 19:11:01 2024 -0500 @@ -101,6 +101,9 @@ msg_list.append(msg) return msg_list # for unittests +# if set to False via interfaces.py do not log a warning when +# xmlrpc is used and defusedxml is not installed. +WARN_FOR_MISSING_DEFUSEDXML = True default_err_msg = ''"""An error has occurred

An error has occurred

@@ -656,6 +659,8 @@ csrf_ok = False # we had an error, failed check if csrf_ok is True: + if WARN_FOR_MISSING_DEFUSEDXML and (not xmlrpc_.client.defusedxml): + logger.warning(self._("XMLRPC endpoint is not using defusedxml. Improve security by installing defusedxml.")) handler = xmlrpc.RoundupDispatcher(self.db, self.instance.actions, self.translator, diff -r 2d0bd038fc5e -r 57325fea9982 test/test_xmlrpc.py --- a/test/test_xmlrpc.py Mon Dec 23 21:10:54 2024 -0500 +++ b/test/test_xmlrpc.py Sun Dec 29 19:11:01 2024 -0500 @@ -5,7 +5,7 @@ # from __future__ import print_function -import unittest, os, shutil, errno, sys, difflib, re +import unittest, os, shutil, errno, pytest, sys, difflib, re from roundup.anypy import xmlrpc_ MultiCall = xmlrpc_.client.MultiCall @@ -21,6 +21,19 @@ from .test_mysql import skip_mysql from .test_postgresql import skip_postgresql +from .pytest_patcher import mark_class +from roundup.anypy.xmlrpc_ import client + +if client.defusedxml: + skip_defusedxml = lambda func, *args, **kwargs: func + + skip_defusedxml_used = mark_class(pytest.mark.skip( + reason='Skipping non-defusedxml tests: defusedxml library in use')) +else: + skip_defusedxml = mark_class(pytest.mark.skip( + reason='Skipping defusedxml tests: defusedxml library not available')) + + skip_defusedxml_used = lambda func, *args, **kwargs: func class XmlrpcTest(object): @@ -314,6 +327,55 @@ for n, r in enumerate(result): self.assertEqual(r, results[n]) + @skip_defusedxml + def testDefusedXmlBomb(self): + self.XmlBomb(expectIn=b"defusedxml.common.EntitiesForbidden") + + @skip_defusedxml_used + def testNonDefusedXmlBomb(self): + self.XmlBomb(expectIn=b"1234567890"*511) + + def XmlBomb(self, expectIn=None): + + bombInput = """ + + + + + ]> + + filter + + + &d; + + + + 0 + 2 + 3 + + + + + + username + demo + + + + + + """ + translator = TranslationService.get_translation( + language=self.instance.config["TRACKER_LANGUAGE"], + tracker_home=self.instance.config["TRACKER_HOME"]) + self.server = RoundupDispatcher(self.db, self.instance.actions, + translator, allow_none = True) + response = self.server.dispatch(bombInput) + print(response) + self.assertIn(expectIn, response) class anydbmXmlrpcTest(XmlrpcTest, unittest.TestCase): backend = 'anydbm'