From d7709f57fdcfa71881518f10245bc9788be9d51b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Diego=20Vel=C3=A1squez?= Date: Wed, 5 Jul 2023 08:21:41 -0500 Subject: [PATCH] chore: remove warnings from defusedxml package --- openedx/core/lib/safe_lxml/__init__.py | 2 - openedx/core/lib/safe_lxml/etree.py | 4 +- openedx/core/lib/safe_lxml/tests.py | 32 +++--- openedx/core/lib/safe_lxml/xmlparser.py | 138 ++++++++++++++++++++++++ xmodule/capa/capa_problem.py | 3 +- 5 files changed, 160 insertions(+), 19 deletions(-) create mode 100644 openedx/core/lib/safe_lxml/xmlparser.py diff --git a/openedx/core/lib/safe_lxml/__init__.py b/openedx/core/lib/safe_lxml/__init__.py index b17efb15cc15..d7d5239c5102 100644 --- a/openedx/core/lib/safe_lxml/__init__.py +++ b/openedx/core/lib/safe_lxml/__init__.py @@ -7,8 +7,6 @@ def defuse_xml_libs(): """ Monkey patch and defuse all stdlib xml packages and lxml. """ - from defusedxml import defuse_stdlib - defuse_stdlib() import lxml import lxml.etree diff --git a/openedx/core/lib/safe_lxml/etree.py b/openedx/core/lib/safe_lxml/etree.py index 5555d1310844..9b685ac66162 100644 --- a/openedx/core/lib/safe_lxml/etree.py +++ b/openedx/core/lib/safe_lxml/etree.py @@ -19,9 +19,7 @@ # These private elements are used in some libraries to also defuse xml exploits for their own purposes. # We need to re-expose them so that the libraries still work. from lxml.etree import _Comment, _Element, _ElementTree, _Entity, _ProcessingInstruction - -# This should be imported after lxml.etree so that it overrides the following attributes. -from defusedxml.lxml import XML, fromstring, parse +from .xmlparser import XML, fromstring, parse class XMLParser(_XMLParser): # pylint: disable=function-redefined diff --git a/openedx/core/lib/safe_lxml/tests.py b/openedx/core/lib/safe_lxml/tests.py index 3608d43bfa93..ceb6b9828124 100644 --- a/openedx/core/lib/safe_lxml/tests.py +++ b/openedx/core/lib/safe_lxml/tests.py @@ -1,29 +1,35 @@ """ Test that we have defused XML. - -For these tests, the defusing will happen in one or more of the `conftest.py` -files that runs at pytest startup calls `defuse_xml_libs()`. - -In production, the defusing happens when the LMS or Studio `wsgi.py` files -call `defuse_xml_libs()`. """ -import defusedxml from lxml import etree +from defusedxml.lxml import EntitiesForbidden +from .xmlparser import fromstring import pytest -@pytest.mark.parametrize("attr", ["XML", "fromstring", "parse"]) -def test_etree_is_defused(attr): - func = getattr(etree, attr) - assert "defused" in func.__code__.co_filename +def test_entities_arent_resolved_exception(): + # Make sure we have disabled entity resolution. + xml = ']> &hi;' + parser = etree.XMLParser() + with pytest.raises(EntitiesForbidden): + _ = etree.XML(xml, parser=parser) + + +def test_entities_resolved(): + xml = ']> &hi;' + parser = etree.XMLParser(resolve_entities=True) + tree = fromstring(xml, parser=parser, forbid_entities=False) + pr = etree.tostring(tree) + assert pr == b'Hello' def test_entities_arent_resolved(): # Make sure we have disabled entity resolution. xml = ']> &hi;' parser = etree.XMLParser() - with pytest.raises(defusedxml.EntitiesForbidden): - _ = etree.XML(xml, parser=parser) + tree = fromstring(xml, parser=parser, forbid_entities=False) + pr = etree.tostring(tree) + assert pr == b'&hi;' diff --git a/openedx/core/lib/safe_lxml/xmlparser.py b/openedx/core/lib/safe_lxml/xmlparser.py new file mode 100644 index 000000000000..b271b43a0fbf --- /dev/null +++ b/openedx/core/lib/safe_lxml/xmlparser.py @@ -0,0 +1,138 @@ +# Based on the lxml example from defusedxml +# +"""lxml.etree protection""" + +from __future__ import print_function, absolute_import + +import threading + +from lxml import etree as _etree + +from defusedxml.lxml import DTDForbidden, EntitiesForbidden, NotSupportedError + +LXML3 = _etree.LXML_VERSION[0] >= 3 + +__origin__ = "lxml.etree" + +tostring = _etree.tostring + + +class RestrictedElement(_etree.ElementBase): + """A restricted Element class that filters out instances of some classes + """ + + __slots__ = () + blacklist = (_etree._Entity, _etree._ProcessingInstruction, _etree._Comment) # pylint: disable=protected-access + + def _filter(self, iterator): # pylint: disable=missing-function-docstring + blacklist = self.blacklist + for child in iterator: + if isinstance(child, blacklist): + continue + yield child + + def __iter__(self): + iterator = super(RestrictedElement, self).__iter__() # pylint: disable=super-with-arguments + return self._filter(iterator) + + def iterchildren(self, tag=None, reversed=False): # pylint: disable=redefined-builtin + iterator = super(RestrictedElement, self).iterchildren(tag=tag, reversed=reversed) # pylint: disable=super-with-arguments + return self._filter(iterator) + + def iter(self, tag=None, *tags): # pylint: disable=keyword-arg-before-vararg + iterator = super(RestrictedElement, self).iter(tag=tag, *tags) # pylint: disable=super-with-arguments + return self._filter(iterator) + + def iterdescendants(self, tag=None, *tags): # pylint: disable=keyword-arg-before-vararg + iterator = super(RestrictedElement, self).iterdescendants(tag=tag, *tags) # pylint: disable=super-with-arguments + return self._filter(iterator) + + def itersiblings(self, tag=None, preceding=False): + iterator = super(RestrictedElement, self).itersiblings(tag=tag, preceding=preceding) # pylint: disable=super-with-arguments + return self._filter(iterator) + + def getchildren(self): + iterator = super(RestrictedElement, self).__iter__() # pylint: disable=super-with-arguments + return list(self._filter(iterator)) + + def getiterator(self, tag=None): + iterator = super(RestrictedElement, self).getiterator(tag) # pylint: disable=super-with-arguments + return self._filter(iterator) + + +class GlobalParserTLS(threading.local): + """Thread local context for custom parser instances + """ + + parser_config = { + "resolve_entities": False, + } + + element_class = RestrictedElement + + def createDefaultParser(self): # pylint: disable=missing-function-docstring + parser = _etree.XMLParser(**self.parser_config) + element_class = self.element_class + if self.element_class is not None: + lookup = _etree.ElementDefaultClassLookup(element=element_class) + parser.set_element_class_lookup(lookup) + return parser + + def setDefaultParser(self, parser): + self._default_parser = parser # pylint: disable=attribute-defined-outside-init + + def getDefaultParser(self): # pylint: disable=missing-function-docstring + parser = getattr(self, "_default_parser", None) + if parser is None: + parser = self.createDefaultParser() + self.setDefaultParser(parser) + return parser + + +_parser_tls = GlobalParserTLS() +getDefaultParser = _parser_tls.getDefaultParser + + +def check_docinfo(elementtree, forbid_dtd=False, forbid_entities=True): + """Check docinfo of an element tree for DTD and entity declarations + The check for entity declarations needs lxml 3 or newer. lxml 2.x does + not support dtd.iterentities(). + """ + docinfo = elementtree.docinfo + if docinfo.doctype: + if forbid_dtd: + raise DTDForbidden(docinfo.doctype, docinfo.system_url, docinfo.public_id) + if forbid_entities and not LXML3: + # lxml < 3 has no iterentities() + raise NotSupportedError("Unable to check for entity declarations " "in lxml 2.x") # pylint: disable=implicit-str-concat + + if forbid_entities: + for dtd in docinfo.internalDTD, docinfo.externalDTD: + if dtd is None: + continue + for entity in dtd.iterentities(): + raise EntitiesForbidden(entity.name, entity.content, None, None, None, None) + + +def parse(source, parser=None, base_url=None, forbid_dtd=False, forbid_entities=True): # pylint: disable=missing-function-docstring + if parser is None: + parser = getDefaultParser() + elementtree = _etree.parse(source, parser, base_url=base_url) + check_docinfo(elementtree, forbid_dtd, forbid_entities) + return elementtree + + +def fromstring(text, parser=None, base_url=None, forbid_dtd=False, forbid_entities=True): # pylint: disable=missing-function-docstring + if parser is None: + parser = getDefaultParser() + rootelement = _etree.fromstring(text, parser, base_url=base_url) + elementtree = rootelement.getroottree() + check_docinfo(elementtree, forbid_dtd, forbid_entities) + return rootelement + + +XML = fromstring + + +def iterparse(*args, **kwargs): + raise NotSupportedError("iterparse not available") diff --git a/xmodule/capa/capa_problem.py b/xmodule/capa/capa_problem.py index d8b9b85c7b85..6f4acc5ad1e8 100644 --- a/xmodule/capa/capa_problem.py +++ b/xmodule/capa/capa_problem.py @@ -36,6 +36,7 @@ from xmodule.capa.safe_exec import safe_exec from xmodule.capa.util import contextualize_text, convert_files_to_filenames, get_course_id_from_capa_block from openedx.core.djangolib.markup import HTML, Text +from openedx.core.lib.safe_lxml.xmlparser import XML from xmodule.stringify import stringify_children # extra things displayed after "show answers" is pressed @@ -184,7 +185,7 @@ def __init__(self, problem_text, id, capa_system, capa_block, # pylint: disable if isinstance(problem_text, six.text_type): # etree chokes on Unicode XML with an encoding declaration problem_text = problem_text.encode('utf-8') - self.tree = etree.XML(problem_text) + self.tree = XML(problem_text) try: self.make_xml_compatible(self.tree)