Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use defusedxml to prevent external entity exploits #94

Merged
merged 15 commits into from
Jul 2, 2022
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