From 96559bfd0f730be8a2e409a38952f92a0142d335 Mon Sep 17 00:00:00 2001
From: Yu Ming Zhu <yzhu@redhat.com>
Date: Jan 15 2024 13:38:41 +0000
Subject: [PATCH 1/3] Bandit [B411]: use defusedxml to prevent remote XML attacks


- putting xmlrpc stuff into koji.xmlrpcplus
- adding koji.xmlrpcplus.xmlrpc_server to refer
- replacing refs of original xmlrpc.client.dumps to enhanced
  koji.xmlrpcplus.dumps

fixes: #3964

---

diff --git a/cli/koji_cli/lib.py b/cli/koji_cli/lib.py
index c605940..a844d21 100644
--- a/cli/koji_cli/lib.py
+++ b/cli/koji_cli/lib.py
@@ -23,7 +23,6 @@ import koji
 from koji import parse_arches
 from koji import _  # noqa: F401
 from koji.util import md5_constructor, to_list
-from koji.xmlrpcplus import xmlrpc_client
 
 
 # for compatibility with plugins based on older version of lib
@@ -208,7 +207,7 @@ class TaskWatcher(object):
         error = None
         try:
             self.session.getTaskResult(self.id)
-        except (six.moves.xmlrpc_client.Fault, koji.GenericError) as e:
+        except (koji.xmlrpcplus.Fault, koji.GenericError) as e:
             error = e
         if error is None:
             # print("%s: complete" % self.str())
@@ -922,6 +921,6 @@ def truncate_string(s, length=47):
 
 class DatetimeJSONEncoder(json.JSONEncoder):
     def default(self, o):
-        if isinstance(o, xmlrpc_client.DateTime):
+        if isinstance(o, koji.xmlrpcplus.DateTime):
             return str(o)
         return json.JSONEncoder.default(self, o)
diff --git a/koji/__init__.py b/koji/__init__.py
index 334b403..a143a4c 100644
--- a/koji/__init__.py
+++ b/koji/__init__.py
@@ -69,7 +69,7 @@ from requests.packages.urllib3.exceptions import MaxRetryError, HostChangedError
 from six.moves import range, zip
 
 from koji.tasks import parse_task_params
-from koji.xmlrpcplus import Fault, dumps, getparser, loads, xmlrpc_client
+from koji.xmlrpcplus import DateTime, Fault, dumps, getparser, loads
 from koji.util import deprecated
 from . import util
 from . import _version
@@ -3661,7 +3661,7 @@ def formatTime(value):
     """Format a timestamp so it looks nicer"""
     if not value and not isinstance(value, (int, float)):
         return ''
-    if isinstance(value, xmlrpc_client.DateTime):
+    if isinstance(value, DateTime):
         value = datetime.datetime.strptime(value.value, "%Y%m%dT%H:%M:%S")
     elif isinstance(value, (int, float)):
         value = datetime.datetime.fromtimestamp(value)
@@ -3684,7 +3684,7 @@ def formatTimeLong(value):
         return ''
     if isinstance(value, six.string_types):
         t = dateutil.parser.parse(value)
-    elif isinstance(value, xmlrpc_client.DateTime):
+    elif isinstance(value, DateTime):
         t = dateutil.parser.parse(value.value)
     elif isinstance(value, (int, float)):
         t = datetime.datetime.fromtimestamp(value)
diff --git a/koji/xmlrpcplus.py b/koji/xmlrpcplus.py
index 7da1593..0999e38 100644
--- a/koji/xmlrpcplus.py
+++ b/koji/xmlrpcplus.py
@@ -6,9 +6,16 @@ from __future__ import absolute_import
 
 import types
 
+import defusedxml.xmlrpc as defusedxmlrpc
 import re
 import six
+# importing here for references in koji by defused.xmlrpc.monkey_patch() below
 import six.moves.xmlrpc_client as xmlrpc_client
+import six.moves.xmlrpc_server as xmlrpc_server  # noqa: F401
+
+
+# patching xmlrpc to protect against XML related attacks
+defusedxmlrpc.monkey_patch()
 
 # duplicate a few values that we need
 getparser = xmlrpc_client.getparser
diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py
index 54d8b68..918793c 100644
--- a/kojihub/kojihub.py
+++ b/kojihub/kojihub.py
@@ -47,7 +47,6 @@ import time
 import types
 import traceback
 from urllib.parse import parse_qs
-import xmlrpc.client
 import zipfile
 from collections import defaultdict, OrderedDict
 
@@ -513,7 +512,7 @@ class Task(object):
             # handle older base64 encoded data
             xml_request = base64.b64decode(xml_request)
         # note: loads accepts either bytes or string
-        params, method = xmlrpc.client.loads(xml_request)
+        params, method = koji.xmlrpcplus.loads(xml_request)
         return params
 
     def getResult(self, raise_fault=True):
@@ -534,8 +533,8 @@ class Task(object):
         try:
             # If the result is a Fault, then loads will raise it
             # This is normally what we want to happen
-            result, method = xmlrpc.client.loads(xml_result)
-        except xmlrpc.client.Fault as fault:
+            result, method = koji.xmlrpcplus.loads(xml_result)
+        except koji.xmlrpcplus.Fault as fault:
             if raise_fault:
                 raise
             # Note that you can't really return a fault over xmlrpc, except by
@@ -573,7 +572,7 @@ class Task(object):
                     # handle older base64 encoded data
                     task['request'] = base64.b64decode(task['request'])
                 # note: loads accepts either bytes or string
-                task['request'] = xmlrpc.client.loads(task['request'])[0]
+                task['request'] = koji.xmlrpcplus.loads(task['request'])[0]
         return results
 
     def runCallbacks(self, cbtype, old_info, attr, new_val):
@@ -13471,8 +13470,8 @@ class RootExports(object):
                             # handle older base64 encoded data
                             val = base64.b64decode(val)
                         # note: loads accepts either bytes or string
-                        data, method = xmlrpc.client.loads(val)
-                    except xmlrpc.client.Fault as fault:
+                        data, method = koji.xmlrpcplus.loads(val)
+                    except koji.xmlrpcplus.Fault as fault:
                         data = fault
                     task[f] = data
             yield task
@@ -13793,7 +13792,7 @@ class RootExports(object):
             xmlrpc DateTime value"""
         context.session.assertPerm('admin')
         buildinfo = get_build(build, strict=True)
-        if isinstance(ts, xmlrpc.client.DateTime):
+        if isinstance(ts, koji.xmlrpcplus.DateTime):
             # not recommended
             # the xmlrpclib.DateTime class is almost useless
             try:
diff --git a/requirements.txt b/requirements.txt
index af34dc6..522f27c 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -6,3 +6,4 @@ python-multilib
 python-qpid-proton<0.38.0;python_version < '3.0'
 python-qpid-proton;python_version >= '3.0'
 rpm-py-installer
+defusedxml
diff --git a/util/koji-gc b/util/koji-gc
index 071203a..134fede 100755
--- a/util/koji-gc
+++ b/util/koji-gc
@@ -15,7 +15,6 @@ import pprint
 import smtplib
 import sys
 import time
-import xmlrpc.client  # for Fault
 from email.mime import text as MIMEText
 from string import Template
 
@@ -1006,7 +1005,7 @@ def handle_prune():
                             session.untagBuildBypass(taginfo['id'], entry['build_id'],
                                                      force=bypass or is_admin)
                             untagged.setdefault(nvr, {})[tagname] = 1
-                        except (xmlrpc.client.Fault, koji.GenericError) as e:
+                        except (koji.xmlrpcplus.Fault, koji.GenericError) as e:
                             print("Warning: untag operation failed: %s" % e)
                             pass
                 # if action == 'keep' do nothing
@@ -1040,7 +1039,7 @@ def handle_prune():
                 print("Deleting untagged build: %s" % nvr)
                 try:
                     session.deleteBuild(build_id, strict=True)
-                except (xmlrpc.client.Fault, koji.GenericError) as e:
+                except (koji.xmlrpcplus.Fault, koji.GenericError) as e:
                     print("Warning: deletion failed: %s" % e)
                     # server issue
                     pass
diff --git a/util/koji-sidetag-cleanup b/util/koji-sidetag-cleanup
index d2e13c4..0350979 100755
--- a/util/koji-sidetag-cleanup
+++ b/util/koji-sidetag-cleanup
@@ -5,7 +5,6 @@ import configparser
 import datetime
 import optparse
 import os
-import xmlrpc
 
 import koji
 
@@ -124,7 +123,7 @@ def get_options():
 def ensure_connection(session):
     try:
         ret = session.getAPIVersion()
-    except xmlrpc.client.ProtocolError:
+    except koji.xmlrpcplus.xmlrpc_client.ProtocolError:
         error("Unable to connect to server")
     if ret != koji.API_VERSION:
         warn("The server is at API version %d and the client is at %d" % (ret, koji.API_VERSION))
diff --git a/util/koji-sweep-db b/util/koji-sweep-db
index 11a759d..b1565bf 100755
--- a/util/koji-sweep-db
+++ b/util/koji-sweep-db
@@ -1,12 +1,12 @@
 #!/usr/bin/python3
 
 import os
-import xmlrpc.client
 from optparse import OptionParser
 from koji.context import context
 
 import koji
 import kojihub.db
+import koji.xmlrpcplus
 from kojihub.db import DeleteProcessor, QueryProcessor, BulkInsertProcessor
 
 
@@ -79,7 +79,7 @@ def clean_scratch_tasks(cursor, vacuum, test, age):
         task_id = row['id']
         request = row['request']
         try:
-            params, _ = xmlrpc.client.loads(request)
+            params, _ = koji.xmlrpcplus.xmlrpc_client.loads(request)
             opts = params[2]
             if opts['scratch']:
                 ids.append(task_id)
diff --git a/vm/kojikamid.py b/vm/kojikamid.py
index fe271e2..70a929f 100755
--- a/vm/kojikamid.py
+++ b/vm/kojikamid.py
@@ -38,7 +38,6 @@ import tempfile
 import threading
 import time
 import traceback
-import xmlrpc.client
 # urllib is required by the SCM class which is substituted into this file
 # do not remove the import below
 import urllib  # noqa: F401
@@ -46,6 +45,7 @@ import zipfile
 from configparser import RawConfigParser
 from optparse import OptionParser
 
+from defusedxml import xmlrpc
 import six    # noqa: F401, needed for imported code
 
 
@@ -53,6 +53,9 @@ MANAGER_PORT = 7000
 
 KOJIKAMID = True
 
+# patching xmlrpc to protect against XML related attacks
+xmlrpc.monkey_patch()
+
 # INSERT kojikamid dup #
 
 
@@ -635,7 +638,9 @@ def get_mgmt_server():
         macaddr, gateway = find_net_info()
     logger.debug('found MAC address %s, connecting to %s:%s',
                  macaddr, gateway, MANAGER_PORT)
-    server = xmlrpc.client.ServerProxy('http://%s:%s/' % (gateway, MANAGER_PORT), allow_none=True)
+    server = xmlrpc.xmlrpc_client.ServerProxy(
+        'http://%s:%s/' % (gateway, MANAGER_PORT), allow_none=True
+    )
     # we would set a timeout on the socket here, but that is apparently not
     # supported by python/cygwin/Windows
     task_port = server.getPort(macaddr)
diff --git a/vm/kojivmd b/vm/kojivmd
index d6c0157..7ddc8b2 100755
--- a/vm/kojivmd
+++ b/vm/kojivmd
@@ -35,8 +35,6 @@ import subprocess
 import sys
 import threading
 import time
-import xmlrpc
-import xmlrpc.server
 from contextlib import closing
 from optparse import OptionParser
 
@@ -45,6 +43,7 @@ import libxml2
 import requests
 
 import koji
+from koji.xmlrpcplus import dumps, Fault, loads, xmlrpc_client, xmlrpc_server
 import koji.util
 from koji.daemon import SCM, TaskManager
 # TaskHandlers are required to be imported, do not remove them
@@ -295,15 +294,15 @@ def main(options, session):
 # Tasks for handling VM lifecycle
 ####################
 
-class DaemonXMLRPCServer(xmlrpc.server.SimpleXMLRPCServer):
+class DaemonXMLRPCServer(xmlrpc_server.SimpleXMLRPCServer):
     allow_reuse_address = True
 
     def __init__(self, addr, port):
         if sys.version_info[:2] <= (2, 4):
-            xmlrpc.server.SimpleXMLRPCServer.__init__(self, (addr, port),
+            xmlrpc_server.SimpleXMLRPCServer.__init__(self, (addr, port),
                                                       logRequests=False)
         else:
-            xmlrpc.server.SimpleXMLRPCServer.__init__(self, (addr, port),
+            xmlrpc_server.SimpleXMLRPCServer.__init__(self, (addr, port),
                                                       logRequests=False,
                                                       allow_none=True)
         self.logger = logging.getLogger('koji.vm.DaemonXMLRPCServer')
@@ -312,7 +311,7 @@ class DaemonXMLRPCServer(xmlrpc.server.SimpleXMLRPCServer):
 
     def server_close(self):
         self.active = False
-        xmlrpc.server.SimpleXMLRPCServer.server_close(self)
+        xmlrpc_server.SimpleXMLRPCServer.server_close(self)
 
     def handle_while_active(self):
         while self.active:
@@ -333,20 +332,20 @@ class DaemonXMLRPCServer(xmlrpc.server.SimpleXMLRPCServer):
         # Copy and paste from SimpleXMLRPCServer, with the addition of passing
         # allow_none=True to xmlrpclib.dumps()
         def _marshaled_dispatch(self, data, dispatch_method=None):
-            params, method = xmlrpc.client.loads(data)
+            params, method = loads(data)
             try:
                 if dispatch_method is not None:
                     response = dispatch_method(method, params)
                 else:
                     response = self._dispatch(method, params)
                 response = (response,)
-                response = xmlrpc.client.dumps(response, methodresponse=1, allow_none=True)
-            except xmlrpc.client.Fault as fault:
-                response = xmlrpc.client.dumps(fault)
+                response = dumps(response, methodresponse=1, allow_none=True)
+            except Fault as fault:
+                response = dumps(fault)
             except Exception:
                 # report exception back to server
-                response = xmlrpc.client.dumps(
-                    xmlrpc.client.Fault(1, "%s:%s" % (sys.exc_info()[0], sys.exc_info()[1]))
+                response = dumps(
+                    Fault(1, "%s:%s" % (sys.exc_info()[0], sys.exc_info()[1]))
                 )
             return response
 
@@ -480,7 +479,7 @@ class VMExecTask(BaseTaskHandler):
 
     def __init__(self, *args, **kw):
         super(VMExecTask, self).__init__(*args, **kw)
-        self.task_manager = xmlrpc.client.ServerProxy(
+        self.task_manager = xmlrpc_client.ServerProxy(
             'http://%s:%s/' % (self.options.privaddr, self.options.portbase), allow_none=True)
         self.port = None
         self.server = None
diff --git a/www/lib/kojiweb/util.py b/www/lib/kojiweb/util.py
index 014add4..301951b 100644
--- a/www/lib/kojiweb/util.py
+++ b/www/lib/kojiweb/util.py
@@ -26,7 +26,6 @@ import re
 import ssl
 import stat
 import urllib
-import xmlrpc.client
 # a bunch of exception classes that explainError needs
 from socket import error as socket_error
 from xml.parsers.expat import ExpatError
@@ -35,6 +34,7 @@ import Cheetah.Template
 
 import koji
 import koji.tasks
+from koji.xmlrpcplus import xmlrpc_client
 
 
 themeInfo = {}
@@ -726,7 +726,7 @@ a bug or a configuration issue."""
 The web interface is having difficulty communicating with the main \
 server. This most likely indicates a network issue."""
         level = 1
-    elif isinstance(error, (xmlrpc.client.ProtocolError, ExpatError)):
+    elif isinstance(error, (xmlrpc_client.ProtocolError, ExpatError)):
         str = """\
 The main server returned an invalid response. This could be caused by \
 a network issue or load issues on the server."""

From 2221bf5b19195de8e1a7d1ea34cf4d89e0b92397 Mon Sep 17 00:00:00 2001
From: Yu Ming Zhu <yzhu@redhat.com>
Date: Jan 15 2024 14:24:06 +0000
Subject: [PATCH 2/3] update spec


---

diff --git a/koji.spec b/koji.spec
index c973233..24785d0 100644
--- a/koji.spec
+++ b/koji.spec
@@ -134,6 +134,7 @@ Requires: python-requests
 Requires: python-requests-gssapi
 Requires: python-dateutil
 Requires: python-six
+Requires: python-defusedxml
 
 %description -n python2-%{name}
 desc
@@ -161,6 +162,7 @@ Requires: python%{python3_pkgversion}-requests-kerberos
 %endif
 Requires: python%{python3_pkgversion}-dateutil
 Requires: python%{python3_pkgversion}-six
+Requires: python%{python3_pkgversion}-defusedxml
 # Since we don't have metadata here, provide the 'normal' python provides manually.
 Provides: python%{python3_version}dist(%{name}) = %{version}
 Provides: python%{python3_pkgversion}dist(%{name}) = %{version}

From 1c7e91309335d28ff8d4754137265bc4283c25e5 Mon Sep 17 00:00:00 2001
From: Yu Ming Zhu <yzhu@redhat.com>
Date: Jan 18 2024 14:32:38 +0000
Subject: [PATCH 3/3] add defusedxml installation in Dockerfiles


---

diff --git a/devtools/containers/Dockerfile.centos7 b/devtools/containers/Dockerfile.centos7
index c81323a..5bdf648 100644
--- a/devtools/containers/Dockerfile.centos7
+++ b/devtools/containers/Dockerfile.centos7
@@ -21,6 +21,7 @@ RUN \
     yum-utils && \
   yum install -y epel-release && \
   yum install -y \
+    python-defusedxml \
     python-flake8 \
     python-multilib \
     python-pip \
diff --git a/devtools/containers/Dockerfile.centos8 b/devtools/containers/Dockerfile.centos8
index aafdab3..70560ff 100644
--- a/devtools/containers/Dockerfile.centos8
+++ b/devtools/containers/Dockerfile.centos8
@@ -14,6 +14,7 @@ RUN \
   libxml2-devel \
   make \
   openssl-devel \
+  python3-defusedxml \
   python3-devel \
   python3-librepo \
   python3-pip \
diff --git a/devtools/containers/Dockerfile.centos9 b/devtools/containers/Dockerfile.centos9
index 790fec6..0c48502 100644
--- a/devtools/containers/Dockerfile.centos9
+++ b/devtools/containers/Dockerfile.centos9
@@ -14,6 +14,7 @@ RUN \
   libxml2-devel \
   make \
   openssl-devel \
+  python3-defusedxml \
   python3-devel \
   python3-librepo \
   python3-pip \
diff --git a/devtools/containers/Dockerfile.f34 b/devtools/containers/Dockerfile.f34
index 93991bc..087264e 100644
--- a/devtools/containers/Dockerfile.f34
+++ b/devtools/containers/Dockerfile.f34
@@ -13,6 +13,7 @@ RUN \
   libxml2-devel \
   make \
   openssl-devel \
+  python3-defusedxml \
   python3-devel \
   python3-pip \
   python3-pytest \
diff --git a/devtools/containers/Dockerfile.f35 b/devtools/containers/Dockerfile.f35
index a7e7894..00ee724 100644
--- a/devtools/containers/Dockerfile.f35
+++ b/devtools/containers/Dockerfile.f35
@@ -13,6 +13,7 @@ RUN \
   libxml2-devel \
   make \
   openssl-devel \
+  python3-defusedxml \
   python3-devel \
   python3-pip \
   python3-pytest \
diff --git a/devtools/containers/Dockerfile.f36 b/devtools/containers/Dockerfile.f36
index 434e729..4b7e04b 100644
--- a/devtools/containers/Dockerfile.f36
+++ b/devtools/containers/Dockerfile.f36
@@ -13,6 +13,7 @@ RUN \
   libxml2-devel \
   make \
   openssl-devel \
+  python3-defusedxml \
   python3-devel \
   python3-pip \
   python3-pytest \
diff --git a/devtools/containers/Dockerfile.f37 b/devtools/containers/Dockerfile.f37
index de75eea..4a86c2e 100644
--- a/devtools/containers/Dockerfile.f37
+++ b/devtools/containers/Dockerfile.f37
@@ -13,6 +13,7 @@ RUN \
   libxml2-devel \
   make \
   openssl-devel \
+  python3-defusedxml \
   python3-devel \
   python3-pip \
   python3-pytest \
diff --git a/devtools/containers/Dockerfile.f38 b/devtools/containers/Dockerfile.f38
index 3253131..05c8ff3 100644
--- a/devtools/containers/Dockerfile.f38
+++ b/devtools/containers/Dockerfile.f38
@@ -13,6 +13,7 @@ RUN \
   libxml2-devel \
   make \
   openssl-devel \
+  python3-defusedxml \
   python3-devel \
   python3-pip \
   python3-pytest \
diff --git a/devtools/containers/Dockerfile.f39 b/devtools/containers/Dockerfile.f39
index 0bb35ef..9a8ba14 100644
--- a/devtools/containers/Dockerfile.f39
+++ b/devtools/containers/Dockerfile.f39
@@ -13,6 +13,7 @@ RUN \
   libxml2-devel \
   make \
   openssl-devel \
+  python3-defusedxml \
   python3-devel \
   python3-pip \
   python3-pytest \
diff --git a/devtools/containers/Dockerfile.rawhide b/devtools/containers/Dockerfile.rawhide
index 7a1f153..09c87b5 100644
--- a/devtools/containers/Dockerfile.rawhide
+++ b/devtools/containers/Dockerfile.rawhide
@@ -13,6 +13,7 @@ RUN \
   libxml2-devel \
   make \
   openssl-devel \
+  python3-defusedxml \
   python3-devel \
   python3-pip \
   python3-pytest \

