CVE-2022-2309: NULL Pointer Dereference (CWE-476)
value: NULL Pointer Dereference allows attackers to cause a denial of service (or application crash). This only applies when lxml is used together with libxml2 2.9.10 through 2.9.14. libxml2 2.9.9 and earlier are not affected. It allows triggering crashes through forged input data, given a vulnerable code sequence in the application. The vulnerability is caused by the iterwalk function (also used by the canonicalize function). Such code shouldn't be in wide-spread use, given that parsing + iterwalk would usually be replaced with the more efficient iterparse function. However, an XML converter that serialises to C14N would also be vulnerable, for example, and there are legitimate use cases for this code sequence. If untrusted input is received (also remotely) and processed via iterwalk function, a crash can be triggered.
CWE description: The product dereferences a pointer that it expects to be valid but is NULL.
CVSS v3 base score: 7.5
Published: 2022-07-05T10:15Z
Repository: https://github.com/lxml/lxml
Language: Other
Files changed: src/lxml/apihelpers.pxi, src/lxml/iterparse.pxi, src/lxml/tests/test_etree.py
Commit date: 2022-07-01 21:16:47 +0200
Commit message: Fix a crash when incorrect parser input occurs together with usages of iterwalk() on trees generated by the same parser.

Diff: diff --git a/src/lxml/apihelpers.pxi b/src/lxml/apihelpers.pxi
index c16627629..9fae9fb12 100644
--- a/src/lxml/apihelpers.pxi
+++ b/src/lxml/apihelpers.pxi
@@ -244,13 +244,14 @@ cdef dict _build_nsmap(xmlNode* c_node):
     cdef xmlNs* c_ns
     nsmap = {}
     while c_node is not NULL and c_node.type == tree.XML_ELEMENT_NODE:
         c_ns = c_node.nsDef
         while c_ns is not NULL:
-            prefix = funicodeOrNone(c_ns.prefix)
-            if prefix not in nsmap:
-                nsmap[prefix] = funicodeOrNone(c_ns.href)
+            if c_ns.prefix or c_ns.href:
+                prefix = funicodeOrNone(c_ns.prefix)
+                if prefix not in nsmap:
+                    nsmap[prefix] = funicodeOrNone(c_ns.href)
             c_ns = c_ns.next
         c_node = c_node.parent
     return nsmap
 
 
diff --git a/src/lxml/iterparse.pxi b/src/lxml/iterparse.pxi
index 138c23a6a..a7299da6d 100644
--- a/src/lxml/iterparse.pxi
+++ b/src/lxml/iterparse.pxi
@@ -418,22 +418,23 @@ cdef int _countNsDefs(xmlNode* c_node):
     cdef xmlNs* c_ns
     cdef int count
     count = 0
     c_ns = c_node.nsDef
     while c_ns is not NULL:
-        count += 1
+        count += (c_ns.href is not NULL)
         c_ns = c_ns.next
     return count
 
 
 cdef int _appendStartNsEvents(xmlNode* c_node, list event_list) except -1:
     cdef xmlNs* c_ns
     cdef int count
     count = 0
     c_ns = c_node.nsDef
     while c_ns is not NULL:
-        ns_tuple = (funicode(c_ns.prefix) if c_ns.prefix is not NULL else '',
-                    funicode(c_ns.href))
-        event_list.append( (u"start-ns", ns_tuple) )
-        count += 1
+        if c_ns.href:
+            ns_tuple = (funicodeOrEmpty(c_ns.prefix),
+                        funicode(c_ns.href))
+            event_list.append( (u"start-ns", ns_tuple) )
+            count += 1
         c_ns = c_ns.next
     return count
diff --git a/src/lxml/tests/test_etree.py b/src/lxml/tests/test_etree.py
index e5f084692..285313f6e 100644
--- a/src/lxml/tests/test_etree.py
+++ b/src/lxml/tests/test_etree.py
@@ -1458,10 +1458,30 @@ class ETreeOnlyTestCase(HelperTestCase):
             counts.append(len(list(elem.getiterator())))
         self.assertEqual(
             [1,2,1,4],
             counts)
 
+    def test_walk_after_parse_failure(self):
+        # This used to be an issue because libxml2 can leak empty namespaces
+        # between failed parser runs.  iterwalk() failed to handle such a tree.
+        try:
+            etree.XML('''<anot xmlns="1">''')
+        except etree.XMLSyntaxError:
+            pass
+        else:
+            assert False, "invalid input did not fail to parse"
+
+        et = etree.XML('''<root>  </root>''')
+        try:
+            ns = next(etree.iterwalk(et, events=('start-ns',)))
+        except StopIteration:
+            # This would be the expected result, because there was no namespace
+            pass
+        else:
+            # This is a bug in libxml2
+            assert not ns, repr(ns)
+
     def test_itertext_comment_pi(self):
         # https://bugs.launchpad.net/lxml/+bug/1844674
         XML = self.etree.XML
         root = XML(_bytes(
             "<root>RTEXT<a></a>ATAIL<b/><!-- COMMENT -->CTAIL<?PI PITEXT?> PITAIL </root>"

Vulnerable code:             prefix = funicodeOrNone(c_ns.prefix)
            if prefix not in nsmap:
                nsmap[prefix] = funicodeOrNone(c_ns.href)
        count += 1
        ns_tuple = (funicode(c_ns.prefix) if c_ns.prefix is not NULL else '',
                    funicode(c_ns.href))
        event_list.append( (u"start-ns", ns_tuple) )
        count += 1
Fixed code:             if c_ns.prefix or c_ns.href:
                prefix = funicodeOrNone(c_ns.prefix)
                if prefix not in nsmap:
                    nsmap[prefix] = funicodeOrNone(c_ns.href)
        count += (c_ns.href is not NULL)
        if c_ns.href:
            ns_tuple = (funicodeOrEmpty(c_ns.prefix),
                        funicode(c_ns.href))
            event_list.append( (u"start-ns", ns_tuple) )
            count += 1
    def test_walk_after_parse_failure(self):
        # This used to be an issue because libxml2 can leak empty namespaces
        # between failed parser runs.  iterwalk() failed to handle such a tree.
        try:
            etree.XML('''<anot xmlns="1">''')
        except etree.XMLSyntaxError:
            pass
        else:
            assert False, "invalid input did not fail to parse"

        et = etree.XML('''<root>  </root>''')
        try:
            ns = next(etree.iterwalk(et, events=('start-ns',)))
        except StopIteration:
            # This would be the expected result, because there was no namespace
            pass
        else:
            # This is a bug in libxml2
            assert not ns, repr(ns)