From 33c67db3c4e83c6b91dda6002f592e29edca20af Mon Sep 17 00:00:00 2001 From: Jarrod Johnson Date: Wed, 1 Jul 2026 08:28:25 -0400 Subject: [PATCH] Further mitigate potential XML misbehavior Since it turns out we already incurred lxml dependency, use lxml etree instead of xml and mitigate risky xml features beyond blocking the word '!entity' --- .../aiohmi/ipmi/oem/lenovo/config.py | 5 +++-- .../aiohmi/ipmi/oem/lenovo/nextscale.py | 5 +++-- .../confluent/discovery/handlers/smm.py | 5 +++-- .../plugins/hardwaremanagement/cooltera.py | 5 +++-- .../plugins/hardwaremanagement/deltapdu.py | 5 +++-- .../plugins/hardwaremanagement/proxmox.py | 21 ++++++++++++++++--- misc/fixsmmexpiry.py | 7 ++++++- 7 files changed, 39 insertions(+), 14 deletions(-) diff --git a/confluent_server/aiohmi/ipmi/oem/lenovo/config.py b/confluent_server/aiohmi/ipmi/oem/lenovo/config.py index 32177f6c..ebf5f7e9 100644 --- a/confluent_server/aiohmi/ipmi/oem/lenovo/config.py +++ b/confluent_server/aiohmi/ipmi/oem/lenovo/config.py @@ -49,17 +49,18 @@ class Unsupported(Exception): pass def fromstring(inputdata): + parser = etree.XMLParser(resolve_entities=False, no_network=True, huge_tree=False) if b'!entity' in inputdata.lower(): raise Exception('Unsupported XML') try: - return etree.fromstring(inputdata) + return etree.fromstring(inputdata, parser=parser) except etree.XMLSyntaxError: inputdata = bytearray(inputdata.decode('utf8', errors='backslashreplace').encode()) for i in range(len(inputdata)): if inputdata[i] < 0x20 and inputdata[i] not in (9, 0xa, 0xd): inputdata[i] = 63 inputdata = bytes(inputdata) - return etree.fromstring(inputdata) + return etree.fromstring(inputdata, parser=parser) diff --git a/confluent_server/aiohmi/ipmi/oem/lenovo/nextscale.py b/confluent_server/aiohmi/ipmi/oem/lenovo/nextscale.py index 968d9951..2cf1148e 100644 --- a/confluent_server/aiohmi/ipmi/oem/lenovo/nextscale.py +++ b/confluent_server/aiohmi/ipmi/oem/lenovo/nextscale.py @@ -16,7 +16,7 @@ import asyncio import fnmatch import struct import weakref -from xml.etree.ElementTree import fromstring as rfromstring +from lxml import etree import zipfile @@ -50,7 +50,8 @@ psutypes = { def fromstring(inputdata): if b'!entity' in inputdata.lower(): raise Exception('!ENTITY not supported in this interface') - return rfromstring(inputdata) + parser = etree.XMLParser(resolve_entities=False, no_network=True, huge_tree=False) + return etree.fromstring(inputdata, parser=parser) def stringtoboolean(originput, name): diff --git a/confluent_server/confluent/discovery/handlers/smm.py b/confluent_server/confluent/discovery/handlers/smm.py index 22147804..97b777d6 100644 --- a/confluent_server/confluent/discovery/handlers/smm.py +++ b/confluent_server/confluent/discovery/handlers/smm.py @@ -26,7 +26,7 @@ except ImportError: import confluent.netutil as netutil import confluent.util as util -from xml.etree.ElementTree import fromstring as rfromstring +import lxml.etree as etree def fromstring(inputdata): if isinstance(inputdata, bytes): @@ -37,7 +37,8 @@ def fromstring(inputdata): raise Exception('!ENTITY not supported in this interface') # The measures above should filter out the risky facets of xml # We don't need sophisticated feature support - return rfromstring(inputdata) # nosec + parser = etree.XMLParser(resolve_entities=False, no_network=True, huge_tree=False) + return etree.fromstring(inputdata, parser=parser) def fixuuid(baduuid): # SMM dumps it out in hex diff --git a/confluent_server/confluent/plugins/hardwaremanagement/cooltera.py b/confluent_server/confluent/plugins/hardwaremanagement/cooltera.py index 86a24774..2b73c488 100644 --- a/confluent_server/confluent/plugins/hardwaremanagement/cooltera.py +++ b/confluent_server/confluent/plugins/hardwaremanagement/cooltera.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from xml.etree.ElementTree import fromstring as rfromstring +from lxml import etree import confluent.util as util import confluent.messages as msg import confluent.exceptions as exc @@ -90,7 +90,8 @@ def fromstring(inputdata): raise Exception('!ENTITY not supported in this interface') # The measures above should filter out the risky facets of xml # We don't need sophisticated feature support - return rfromstring(inputdata) # nosec + parser = etree.XMLParser(resolve_entities=False, no_network=True, huge_tree=False) + return etree.fromstring(inputdata, parser=parser) # nosec def simplify_name(name): return name.lower().replace(' ', '_').replace('/', '-').replace( diff --git a/confluent_server/confluent/plugins/hardwaremanagement/deltapdu.py b/confluent_server/confluent/plugins/hardwaremanagement/deltapdu.py index 481845c6..58d3b785 100644 --- a/confluent_server/confluent/plugins/hardwaremanagement/deltapdu.py +++ b/confluent_server/confluent/plugins/hardwaremanagement/deltapdu.py @@ -18,9 +18,10 @@ import asyncio import confluent.util as util import confluent.messages as msg import confluent.exceptions as exc -from xml.etree.ElementTree import fromstring as rfromstring +import lxml.etree as etree def fromstring(inputdata): + parser = etree.XMLParser(resolve_entities=False, no_network=True, huge_tree=False) if isinstance(inputdata, bytes): cmpstr = b'!entity' else: @@ -29,7 +30,7 @@ def fromstring(inputdata): raise Exception('!ENTITY not supported in this interface') # The measures above should filter out the risky facets of xml # We don't need sophisticated feature support - return rfromstring(inputdata) # nosec + return etree.fromstring(inputdata, parser=parser) # nosec import http.client as httplib diff --git a/confluent_server/confluent/plugins/hardwaremanagement/proxmox.py b/confluent_server/confluent/plugins/hardwaremanagement/proxmox.py index c47a3cda..50d00064 100644 --- a/confluent_server/confluent/plugins/hardwaremanagement/proxmox.py +++ b/confluent_server/confluent/plugins/hardwaremanagement/proxmox.py @@ -1,5 +1,6 @@ import asyncio +import confluent.exceptions as exc import confluent.vinzmanager as vinzmanager import confluent.util as util import confluent.messages as msg @@ -193,6 +194,8 @@ class PmxApiClient: } loginbody = urlparse.urlencode(loginform) rsp = await self.wc.grab_json_response_with_status('/api2/json/access/ticket', loginbody, headers={'Content-Type': 'application/x-www-form-urlencoded'}) + if rsp[1] == 401: + raise exc.TargetEndpointBadCredentials() self.pac = rsp[0]['data']['ticket'] self.wc.cookies.update_cookies({'PVEAuthCookie': self.pac}) self.wc.set_header('CSRFPreventionToken', rsp[0]['data']['CSRFPreventionToken']) @@ -374,9 +377,12 @@ def prep_proxmox_clients(nodes, configmanager): cfg = cfginfo[node] currpmx = cfg['hardwaremanagement.manager']['value'] if currpmx not in clientsbypmx: - user = cfg.get('secret.hardwaremanagementuser', {}).get('value', None) - passwd = cfg.get('secret.hardwaremanagementpassword', {}).get('value', None) - clientsbypmx[currpmx] = PmxApiClient(currpmx, user, passwd, configmanager) + user = cfg.get('secret.hardwaremanagementuser', {}).get('value', None) + passwd = cfg.get('secret.hardwaremanagementpassword', {}).get('value', None) + try: + clientsbypmx[currpmx] = PmxApiClient(currpmx, user, passwd, configmanager) + except exc.TargetEndpointBadCredentials as e: + clientsbypmx[currpmx] = e clientsbynode[node] = clientsbypmx[currpmx] return clientsbynode @@ -384,6 +390,9 @@ async def retrieve(nodes, element, configmanager, inputdata): clientsbynode = prep_proxmox_clients(nodes, configmanager) for node in nodes: currclient = clientsbynode[node] + if isinstance(currclient, exc.TargetEndpointBadCredentials): + yield msg.ConfluentNodeError(node, "Bad credentials") + continue if element == ['power', 'state']: yield msg.PowerState(node, await currclient.get_vm_power(node)) elif element == ['boot', 'nextdevice']: @@ -405,6 +414,9 @@ async def update(nodes, element, configmanager, inputdata): clientsbynode = prep_proxmox_clients(nodes, configmanager) for node in nodes: currclient = clientsbynode[node] + if isinstance(currclient, exc.TargetEndpointBadCredentials): + yield msg.ConfluentNodeError(node, "Bad credentials") + continue if element == ['power', 'state']: newstate, oldstate = await currclient.set_vm_power(node, inputdata.powerstate(node)) yield msg.PowerState(node, newstate, oldstate) @@ -425,6 +437,9 @@ async def update(nodes, element, configmanager, inputdata): async def create(nodes, element, configmanager, inputdata): clientsbynode = prep_proxmox_clients(nodes, configmanager) for node in nodes: + if isinstance(clientsbynode[node], exc.TargetEndpointBadCredentials): + yield msg.ConfluentNodeError(node, "Bad credentials") + continue if element == ['console', 'ikvm']: try: currclient = clientsbynode[node] diff --git a/misc/fixsmmexpiry.py b/misc/fixsmmexpiry.py index 2e09da97..cada0738 100644 --- a/misc/fixsmmexpiry.py +++ b/misc/fixsmmexpiry.py @@ -1,9 +1,14 @@ #!/usr/bin/python3 import pyghmi.util.webclient as webclient -from xml.etree.ElementTree import fromstring +import lxml.etree as etree import os import sys +def fromstring(inputdata): + parser = etree.XMLParser(resolve_entities=False, no_network=True, huge_tree=False) + # The measures above should filter out the risky facets of xml + return etree.fromstring(inputdata, parser=parser) + tmppassword = 'to3BdS91ABrd' missingargs = False if 'SMMUSER' not in os.environ: