Skip to content

Commit

Permalink
Use defusedxml to prevent external entity exploits (#94)
Browse files Browse the repository at this point in the history
* Use defusedxml to prevent external entity exploits

Fixes #93
  • Loading branch information
stchris authored Jul 2, 2022
1 parent 42e4b3a commit 6f2c105
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 16 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ jobs:
python-version: "3.10"

- name: Install requirements
run: python -m pip install wheel
run: python -m pip install wheel setuptools build

- name: Build a distribution
run: python setup.py sdist bdist_wheel
run: python -m build

- name: Publish package to TestPyPI
uses: pypa/gh-action-pypi-publish@master
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ Changelog
---------

Unreleased
- (SECURITY) Use [defusedxml](https://github.com/tiran/defusedxml) to prevent XML SAX vulnerabilities ([#93](https://github.com/stchris/untangle/issues/93))

1.2.0
- (SECURITY) Prevent XML SAX vulnerability: External Entities injection ([#60](https://github.com/stchris/untangle/issues/60))
Expand Down
1 change: 1 addition & 0 deletions MANIFEST.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include README.md
56 changes: 55 additions & 1 deletion poetry.lock

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

5 changes: 5 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,19 @@ version = "1.2.0"
description = "Converts XML to Python objects"
authors = ["Christian Stefanescu <hello@stchris.net>"]
license = "MIT"
readme = "README.md"

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

[tool.poetry.dev-dependencies]
pytest = "^7.1.2"
flake8 = "^4.0.1"
black = "^22.6.0"
build = "^0.8.0"
setuptools = "^62.6.0"
wheel = "^0.37.1"

[build-system]
requires = ["poetry-core>=1.0.0"]
Expand Down
9 changes: 7 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,24 @@

import untangle

from setuptools import setup
from setuptools import setup, find_packages
from pathlib import Path

long_description = (Path(__file__).parent / "README.md").read_text()

setup(
name="untangle",
packages=find_packages(),
version=untangle.__version__,
description="Convert XML documents into Python objects",
long_description_content_type="text/markdown",
long_description=(Path(__file__).parent / "README.md").read_text(),
long_description=long_description,
author="Christian Stefanescu",
author_email="hello@stchris.net",
url="http://github.com/stchris//untangle",
py_modules=["untangle"],
install_requires=["defusedxml"],
include_package_data=True,
license="MIT",
classifiers=[
"Development Status :: 5 - Production/Stable",
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 6f2c105

Please sign in to comment.