From 051ae0f30b6b584f6416a9b1aa628376613e3c08 Mon Sep 17 00:00:00 2001 From: Abhinav <7419ab@gmail.com> Date: Tue, 22 Sep 2026 19:15:34 +0530 Subject: [PATCH 1/2] fix(userid): parse XML through defusedxml to prevent XXE panos/userid.py built XML objects via the unprotected stdlib xml.etree.ElementTree.fromstring, with no defusedxml/defuse_stdlib usage anywhere in the codebase. Route the two fromstring() call sites in UserId (initial uid-message template, and the registered-user command builder in get_user_tags) through defusedxml.ElementTree instead, so a malicious external entity/billion-laughs payload is rejected rather than resolved. Left the other 9 modules named in the report (base.py, firewall.py, network.py, panorama.py, policies.py, objects.py, ha.py, plugins.py, __init__.py) untouched: they only use ElementTree for outbound construction/serialization (Element/SubElement/tostring), which defusedxml.ElementTree does not provide -- a blind import swap there would break construction, not add any parsing protection, since none of them call fromstring/parse/XML on untrusted input. Added defusedxml as a runtime dependency in pyproject.toml and a regression test proving the parser now in use rejects XXE payloads. poetry.lock/requirements.txt are not regenerated here since our local poetry could not resolve this project's existing pyproject.toml (unrelated docutils/python-range conflict); please run `make sync-deps` on your end if the lockfile needs updating. Fixes part of #617 (Finding 1). Finding 2 (SHA-1 usage in _sha1_hash) is left for maintainers to weigh in on -- that hash format is used to match a specific PAN-OS device API field, so changing the algorithm risks breaking interop with real devices. Co-Authored-By: Claude Sonnet 5 --- panos/userid.py | 5 +++-- pyproject.toml | 1 + tests/test_userid.py | 14 ++++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/panos/userid.py b/panos/userid.py index 675b269e..deffea53 100644 --- a/panos/userid.py +++ b/panos/userid.py @@ -21,6 +21,7 @@ from copy import deepcopy from xml.sax.saxutils import escape, quoteattr +import defusedxml.ElementTree as defused_ET from pan.xapi import PanXapiError import panos.errors as err @@ -62,7 +63,7 @@ def __init__(self, device, prefix="", ignore_dup_errors=True): self.ignore_dup_errors = ignore_dup_errors # Build the initial uid-message - self._uidmessage = ET.fromstring( + self._uidmessage = defused_ET.fromstring( "" + "1.0" + "update" @@ -658,7 +659,7 @@ def get_user_tags(self, user=None, prefix=None): msg.append("{0}".format(escape(user))) msg.append("") - cmd = ET.fromstring("".join(msg)) + cmd = defused_ET.fromstring("".join(msg)) if user is None: start_elm = cmd.find("./object/registered-user/all/start-point") diff --git a/pyproject.toml b/pyproject.toml index 35481fef..1fc3e9f1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,6 +27,7 @@ classifiers = [ [tool.poetry.dependencies] python = "^2.7 || ^3.5" pan-python = "^0.17.0" +defusedxml = "^0.7.1" [tool.poetry.group.dev.dependencies] pytest = {version = "^5.3.2", python = "^3.5"} diff --git a/tests/test_userid.py b/tests/test_userid.py index c2d0bf6d..5d84890a 100644 --- a/tests/test_userid.py +++ b/tests/test_userid.py @@ -19,8 +19,11 @@ import unittest import xml.etree.ElementTree as ET +import defusedxml.common + import panos.firewall import panos.panorama +import panos.userid class TestUserId(unittest.TestCase): @@ -157,6 +160,17 @@ def test_get_group_members_escapes_group(self): self.assertEqual(names[0].text, evil) self.assertIsNone(parsed.find(".//evil")) + def test_userid_uses_defused_xml_parser(self): + # panos.userid must parse XML through defusedxml, not the + # unprotected stdlib parser, to avoid XXE/billion-laughs attacks. + xxe_payload = ( + "" + "]>" + "&xxe;" + ) + with self.assertRaises(defusedxml.common.EntitiesForbidden): + panos.userid.defused_ET.fromstring(xxe_payload) + if __name__ == "__main__": unittest.main() From 0bed28f0205a2faec13d453cbba0ee656e0450fc Mon Sep 17 00:00:00 2001 From: Abhinav <7419ab@gmail.com> Date: Tue, 22 Sep 2026 19:23:00 +0530 Subject: [PATCH 2/2] fix(docs): add defusedxml to docs build requirements docs/conf.py imports panos directly for autodoc, which now transitively imports defusedxml via panos.userid. RTD's build environment installs from docs/requirements.txt independently of pyproject.toml, so it didn't have defusedxml and the docs build failed on import. Also regenerated the root requirements.txt (already declared in pyproject.toml, just needed the pinned/hashed entry) via poetry export. Left poetry.lock alone -- regenerating it with the poetry version available here produced an ~800-line diff across unrelated dependencies (different resolver/poetry version than whatever the maintainers use), so a maintainer should regenerate that one with their own toolchain if needed. Co-Authored-By: Claude Sonnet 5 --- docs/requirements.txt | 1 + requirements.txt | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/requirements.txt b/docs/requirements.txt index 461333f4..c98ca95e 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -2,3 +2,4 @@ mistune<2.0.0 m2r==0.3.1 sphinx_rtd_theme pan-python==0.17.0 +defusedxml==0.7.1 diff --git a/requirements.txt b/requirements.txt index 89baf35c..9e192200 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,6 @@ -pan-python==0.17.0 \ +defusedxml==0.7.1 ; python_version >= "2.7" and python_version < "3.0.dev0" or python_version >= "3.5" and python_version < "4.0" \ + --hash=sha256:1bb3032db185915b62d7c6209c5a8792be6a32ab2fedacc84e01b52c51aa3e69 \ + --hash=sha256:a352e7e428770286cc899e2542b6cdaedb2b4953ff269a210103ec58f6198a61 +pan-python==0.17.0 ; python_version >= "2.7" and python_version < "3.0" or python_version >= "3.5" and python_version < "4.0" \ --hash=sha256:9c074ea2f69a63996a6fefe8935d60dca61660e14715ac19d257ea9b1c41c6e2 \ --hash=sha256:f4674e40763c46d5933244b3059a57884e4e28205ef6d0f9ce2dc2013e3db010