CVE-2021-29421: Improper Restriction of XML External Entity Reference (CWE-611)
value: models/metadata.py in the pikepdf package 1.3.0 through 2.9.2 for Python allows XXE when parsing XMP metadata entries.
CWE description: The product processes an XML document that can contain XML entities with URIs that resolve to documents outside of the intended sphere of control, causing the product to embed incorrect documents into its output.
CVSS v3 base score: 7.5
Published: 2021-04-01T20:15Z
Repository: https://github.com/pikepdf/pikepdf
Language: Python
Files changed: src/pikepdf/_xml.py, src/pikepdf/models/metadata.py, tests/test_metadata.py
Commit date: 2021-03-29 23:59:59 -0700
Commit message: Fix XXE vulnerability in XMP metadata parsing

For details:
https://portswigger.net/web-security/xxe

Reported by: Eric Therond eric.therond@sonarsource.com) of Sonarsource (https://www.sonarsource.com/)

Diff: diff --git a/src/pikepdf/_xml.py b/src/pikepdf/_xml.py
new file mode 100644
index 0000000..f0e1c38
--- /dev/null
+++ b/src/pikepdf/_xml.py
@@ -0,0 +1,30 @@
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# Copyright (C) 2021, James R. Barlow (https://github.com/jbarlow83/)
+
+
+from typing import IO, Any, AnyStr, Union
+
+from lxml.etree import XMLParser as _UnsafeXMLParser
+from lxml.etree import parse as _parse
+
+
+class _XMLParser(_UnsafeXMLParser):
+    def __init__(self, *args, **kwargs):
+        # Prevent XXE attacks
+        # https://rules.sonarsource.com/python/type/Vulnerability/RSPEC-2755
+        kwargs['resolve_entities'] = False
+        kwargs['no_network'] = True
+        super().__init__(*args, **kwargs)
+
+
+def parse_xml(source: Union[AnyStr, IO[Any]], recover: bool = False):
+    """Wrapper around lxml's parse to provide protection against XXE attacks."""
+
+    parser = _XMLParser(recover=recover, remove_pis=False)
+    return _parse(source, parser=parser)
+
+
+__all__ = ['parse_xml']
diff --git a/src/pikepdf/models/metadata.py b/src/pikepdf/models/metadata.py
index 086eed1..9cb2203 100644
--- a/src/pikepdf/models/metadata.py
+++ b/src/pikepdf/models/metadata.py
@@ -24,14 +24,15 @@ from typing import (
     Union,
 )
 from warnings import warn
 
 from lxml import etree
-from lxml.etree import QName, XMLParser, XMLSyntaxError, parse
+from lxml.etree import QName, XMLSyntaxError
 
 from .. import Name, Stream, String
 from .. import __version__ as pikepdf_version
+from .._xml import parse_xml
 
 if sys.version_info < (3, 9):  # pragma: no cover
     from typing import Iterable, MutableMapping
 else:
     from collections.abc import Iterable, MutableMapping
@@ -411,18 +412,17 @@ class PdfMetadata(MutableMapping):
     def _load_from(self, data: bytes) -> None:
         if data.strip() == b'':
             data = XMP_EMPTY  # on some platforms lxml chokes on empty documents
 
         def basic_parser(xml):
-            return parse(BytesIO(xml))
+            return parse_xml(BytesIO(xml))
 
         def strip_illegal_bytes_parser(xml):
-            return parse(BytesIO(re_xml_illegal_bytes.sub(b'', xml)))
+            return parse_xml(BytesIO(re_xml_illegal_bytes.sub(b'', xml)))
 
         def recovery_parser(xml):
-            parser = XMLParser(recover=True)
-            return parse(BytesIO(xml), parser)
+            return parse_xml(BytesIO(xml), recover=True)
 
         def replace_with_empty_xmp(_xml=None):
             log.warning("Error occurred parsing XMP, replacing with empty XMP.")
             return basic_parser(XMP_EMPTY)
 
diff --git a/tests/test_metadata.py b/tests/test_metadata.py
index d27f9a4..d6f35a2 100644
--- a/tests/test_metadata.py
+++ b/tests/test_metadata.py
@@ -727,5 +727,29 @@ def test_exception_undoes_edits(graph):
     except ValueError as e:
         if 'test_exception_aborts_edits' not in str(e):
             raise
         m = graph.open_metadata()
         assert m['dc:format'] != 'application/pdf-demo'
+
+
+def test_xxe(trivial, outdir):
+    secret = outdir / 'secret.txt'
+    secret.write_text("This is a secret")
+    trivial.Root.Metadata = Stream(
+        trivial,
+        b"""\
+<?xpacket begin='\xef\xbb\xbf' id='W5M0MpCehiHzreSzNTczkc9d'?>
+<!DOCTYPE rdf:RDF [<!ENTITY xxe SYSTEM "file://%s">]>
+<x:xmpmeta xmlns:x='adobe:ns:meta/' x:xmptk='Image'>
+<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
+<note>
+<to>&xxe;</to>
+<from>xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx</from>
+</note>
+</rdf:RDF>
+</x:xmpmeta>
+<?xpacket end='w'?>
+    """
+        % os.fsencode(secret),
+    )
+    with trivial.open_metadata() as m:
+        assert 'This is a secret' not in str(m)

Vulnerable code: from lxml.etree import QName, XMLParser, XMLSyntaxError, parse
            return parse(BytesIO(xml))
            return parse(BytesIO(re_xml_illegal_bytes.sub(b'', xml)))
            parser = XMLParser(recover=True)
            return parse(BytesIO(xml), parser)
Fixed code: # This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
#
# Copyright (C) 2021, James R. Barlow (https://github.com/jbarlow83/)


from typing import IO, Any, AnyStr, Union

from lxml.etree import XMLParser as _UnsafeXMLParser
from lxml.etree import parse as _parse


class _XMLParser(_UnsafeXMLParser):
    def __init__(self, *args, **kwargs):
        # Prevent XXE attacks
        # https://rules.sonarsource.com/python/type/Vulnerability/RSPEC-2755
        kwargs['resolve_entities'] = False
        kwargs['no_network'] = True
        super().__init__(*args, **kwargs)


def parse_xml(source: Union[AnyStr, IO[Any]], recover: bool = False):
    """Wrapper around lxml's parse to provide protection against XXE attacks."""

    parser = _XMLParser(recover=recover, remove_pis=False)
    return _parse(source, parser=parser)


__all__ = ['parse_xml']
from lxml.etree import QName, XMLSyntaxError
from .._xml import parse_xml
            return parse_xml(BytesIO(xml))
            return parse_xml(BytesIO(re_xml_illegal_bytes.sub(b'', xml)))
            return parse_xml(BytesIO(xml), recover=True)


def test_xxe(trivial, outdir):
    secret = outdir / 'secret.txt'
    secret.write_text("This is a secret")
    trivial.Root.Metadata = Stream(
        trivial,
        b"""\
<?xpacket begin='\xef\xbb\xbf' id='W5M0MpCehiHzreSzNTczkc9d'?>
<!DOCTYPE rdf:RDF [<!ENTITY xxe SYSTEM "file://%s">]>
<x:xmpmeta xmlns:x='adobe:ns:meta/' x:xmptk='Image'>
<rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
<note>
<to>&xxe;</to>
<from>xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx</from>
</note>
</rdf:RDF>
</x:xmpmeta>
<?xpacket end='w'?>
    """
        % os.fsencode(secret),
    )
    with trivial.open_metadata() as m:
        assert 'This is a secret' not in str(m)