Skip to content

Commit

Permalink
Use defusedxml to prevent external entity exploits
Browse files Browse the repository at this point in the history
Fixes #93
  • Loading branch information
stchris committed Jul 1, 2022
1 parent 42e4b3a commit 04dd876
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 12 deletions.
14 changes: 13 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ license = "MIT"

[tool.poetry.dependencies]
python = "^3.7"
defusedxml = "^0.7.1"

[tool.poetry.dev-dependencies]
pytest = "^7.1.2"
Expand Down
16 changes: 9 additions & 7 deletions tests/test_untangle.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import untangle
import xml

import defusedxml


class FromStringTestCase(unittest.TestCase):
"""Basic parsing tests with input as string"""
Expand Down Expand Up @@ -364,16 +366,16 @@ class ParserFeatureTestCase(unittest.TestCase):
def test_valid_feature(self):
# xml.sax.handler.feature_external_ges -> load external general (text)
# entities, such as DTDs
doc = untangle.parse(self.bad_dtd_xml, feature_external_ges=False)
self.assertEqual(doc.foo["bar"], "baz")
with self.assertRaises(defusedxml.common.ExternalReferenceForbidden):
untangle.parse(self.bad_dtd_xml)

def test_invalid_feature(self):
with self.assertRaises(AttributeError):
untangle.parse(self.bad_dtd_xml, invalid_feature=True)

def test_invalid_external_dtd(self):
with self.assertRaises(IOError):
untangle.parse(self.bad_dtd_xml, feature_external_ges=True)
with self.assertRaises(defusedxml.common.ExternalReferenceForbidden):
untangle.parse(self.bad_dtd_xml)


class TestEquals(unittest.TestCase):
Expand All @@ -393,11 +395,11 @@ def test_list_equals(self):
class TestExternalEntityExpansion(unittest.TestCase):
def test_xxe(self):
# from https://pypi.org/project/defusedxml/#external-entity-expansion-remote
o = untangle.parse("tests/res/xxe.xml")
assert o.root.cdata == ""
with self.assertRaises(defusedxml.common.EntitiesForbidden):
untangle.parse("tests/res/xxe.xml")


if __name__ == "__main__":
unittest.main()

# vim: set expandtab ts=4 sw=4:
# vim: set expandtab ts=4 sw=4
11 changes: 7 additions & 4 deletions untangle.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
"""
import os
import keyword
from xml.sax import make_parser, handler
from xml.sax.handler import feature_external_ges
from defusedxml.sax import make_parser
from xml.sax import handler


try:
Expand Down Expand Up @@ -188,12 +188,15 @@ def parse(filename, **parser_features):
Raises ``xml.sax.SAXParseException`` if something goes wrong
during parsing.
Raises ``defusedxml.common.EntitiesForbidden``
or ``defusedxml.common.ExternalReferenceForbidden``
when a potentially malicious entity load is attempted. See also
https://github.com/tiran/defusedxml#attack-vectors
"""
if filename is None or (is_string(filename) and filename.strip()) == "":
raise ValueError("parse() takes a filename, URL or XML string")
parser = make_parser()
# See https://github.com/stchris/untangle/issues/60
parser.setFeature(feature_external_ges, False)
for feature, value in parser_features.items():
parser.setFeature(getattr(handler, feature), value)
sax_handler = Handler()
Expand Down

0 comments on commit 04dd876

Please sign in to comment.