From 752e5f71d7cea2ca5a7e7c0b8f72ed04ce654be4 Mon Sep 17 00:00:00 2001 From: Nick Wellnhofer Date: Wed, 10 Jun 2020 16:34:52 +0200 Subject: [PATCH 1/2] Don't recurse into xi:include children in xmlXIncludeDoProcess Otherwise, nested xi:include nodes might result in a use-after-free if XML_PARSE_NOXINCNODE is specified. Found with libFuzzer and ASan. --- result/XInclude/fallback3.xml | 8 ++++++++ result/XInclude/fallback3.xml.err | 0 result/XInclude/fallback3.xml.rdr | 25 +++++++++++++++++++++++++ result/XInclude/fallback4.xml | 10 ++++++++++ result/XInclude/fallback4.xml.err | 0 result/XInclude/fallback4.xml.rdr | 29 +++++++++++++++++++++++++++++ test/XInclude/docs/fallback3.xml | 9 +++++++++ test/XInclude/docs/fallback4.xml | 7 +++++++ xinclude.c | 24 ++++++++++-------------- 9 files changed, 98 insertions(+), 14 deletions(-) create mode 100644 result/XInclude/fallback3.xml create mode 100644 result/XInclude/fallback3.xml.err create mode 100644 result/XInclude/fallback3.xml.rdr create mode 100644 result/XInclude/fallback4.xml create mode 100644 result/XInclude/fallback4.xml.err create mode 100644 result/XInclude/fallback4.xml.rdr create mode 100644 test/XInclude/docs/fallback3.xml create mode 100644 test/XInclude/docs/fallback4.xml diff --git a/result/XInclude/fallback3.xml b/result/XInclude/fallback3.xml new file mode 100644 index 00000000..b4235514 --- /dev/null +++ b/result/XInclude/fallback3.xml @@ -0,0 +1,8 @@ + + + +

something

+

really

+

simple

+
+
diff --git a/result/XInclude/fallback3.xml.err b/result/XInclude/fallback3.xml.err new file mode 100644 index 00000000..e69de29b diff --git a/result/XInclude/fallback3.xml.rdr b/result/XInclude/fallback3.xml.rdr new file mode 100644 index 00000000..aa2f1374 --- /dev/null +++ b/result/XInclude/fallback3.xml.rdr @@ -0,0 +1,25 @@ +0 1 a 0 0 +1 14 #text 0 1 + +1 1 doc 0 0 +2 14 #text 0 1 + +2 1 p 0 0 +3 3 #text 0 1 something +2 15 p 0 0 +2 14 #text 0 1 + +2 1 p 0 0 +3 3 #text 0 1 really +2 15 p 0 0 +2 14 #text 0 1 + +2 1 p 0 0 +3 3 #text 0 1 simple +2 15 p 0 0 +2 14 #text 0 1 + +1 15 doc 0 0 +1 14 #text 0 1 + +0 15 a 0 0 diff --git a/result/XInclude/fallback4.xml b/result/XInclude/fallback4.xml new file mode 100644 index 00000000..9883fd54 --- /dev/null +++ b/result/XInclude/fallback4.xml @@ -0,0 +1,10 @@ + + + + +

something

+

really

+

simple

+
+ +
diff --git a/result/XInclude/fallback4.xml.err b/result/XInclude/fallback4.xml.err new file mode 100644 index 00000000..e69de29b diff --git a/result/XInclude/fallback4.xml.rdr b/result/XInclude/fallback4.xml.rdr new file mode 100644 index 00000000..628b9513 --- /dev/null +++ b/result/XInclude/fallback4.xml.rdr @@ -0,0 +1,29 @@ +0 1 a 0 0 +1 14 #text 0 1 + +1 14 #text 0 1 + +1 1 doc 0 0 +2 14 #text 0 1 + +2 1 p 0 0 +3 3 #text 0 1 something +2 15 p 0 0 +2 14 #text 0 1 + +2 1 p 0 0 +3 3 #text 0 1 really +2 15 p 0 0 +2 14 #text 0 1 + +2 1 p 0 0 +3 3 #text 0 1 simple +2 15 p 0 0 +2 14 #text 0 1 + +1 15 doc 0 0 +1 14 #text 0 1 + +1 14 #text 0 1 + +0 15 a 0 0 diff --git a/test/XInclude/docs/fallback3.xml b/test/XInclude/docs/fallback3.xml new file mode 100644 index 00000000..0c8b6c9e --- /dev/null +++ b/test/XInclude/docs/fallback3.xml @@ -0,0 +1,9 @@ + + + + + There is no c.xml ... + + + + diff --git a/test/XInclude/docs/fallback4.xml b/test/XInclude/docs/fallback4.xml new file mode 100644 index 00000000..b500a635 --- /dev/null +++ b/test/XInclude/docs/fallback4.xml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/xinclude.c b/xinclude.c index ba850fa5..f260c1a7 100644 --- a/xinclude.c +++ b/xinclude.c @@ -2392,21 +2392,19 @@ xmlXIncludeDoProcess(xmlXIncludeCtxtPtr ctxt, xmlDocPtr doc, xmlNodePtr tree) { * First phase: lookup the elements in the document */ cur = tree; - if (xmlXIncludeTestNode(ctxt, cur) == 1) - xmlXIncludePreProcessNode(ctxt, cur); while ((cur != NULL) && (cur != tree->parent)) { /* TODO: need to work on entities -> stack */ - if ((cur->children != NULL) && - (cur->children->type != XML_ENTITY_DECL) && - (cur->children->type != XML_XINCLUDE_START) && - (cur->children->type != XML_XINCLUDE_END)) { - cur = cur->children; - if (xmlXIncludeTestNode(ctxt, cur)) - xmlXIncludePreProcessNode(ctxt, cur); - } else if (cur->next != NULL) { + if (xmlXIncludeTestNode(ctxt, cur) == 1) { + xmlXIncludePreProcessNode(ctxt, cur); + } else if ((cur->children != NULL) && + (cur->children->type != XML_ENTITY_DECL) && + (cur->children->type != XML_XINCLUDE_START) && + (cur->children->type != XML_XINCLUDE_END)) { + cur = cur->children; + continue; + } + if (cur->next != NULL) { cur = cur->next; - if (xmlXIncludeTestNode(ctxt, cur)) - xmlXIncludePreProcessNode(ctxt, cur); } else { if (cur == tree) break; @@ -2416,8 +2414,6 @@ xmlXIncludeDoProcess(xmlXIncludeCtxtPtr ctxt, xmlDocPtr doc, xmlNodePtr tree) { break; /* do */ if (cur->next != NULL) { cur = cur->next; - if (xmlXIncludeTestNode(ctxt, cur)) - xmlXIncludePreProcessNode(ctxt, cur); break; /* do */ } } while (cur != NULL); -- 2.31.1 From 49cc4182543dba73216add4021994a81678763bd Mon Sep 17 00:00:00 2001 From: Nick Wellnhofer Date: Thu, 22 Apr 2021 19:26:28 +0200 Subject: [PATCH 2/2] Fix user-after-free with `xmllint --xinclude --dropdtd` The --dropdtd option can leave dangling pointers in entity reference nodes. Make sure to skip these nodes when processing XIncludes. This also avoids scanning entity declarations and even modifying them inadvertently during XInclude processing. Move from a block list to an allow list approach to avoid descending into other node types that can't contain elements. Fixes #237. --- xinclude.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/xinclude.c b/xinclude.c index f260c1a7..d7648529 100644 --- a/xinclude.c +++ b/xinclude.c @@ -2397,9 +2397,8 @@ xmlXIncludeDoProcess(xmlXIncludeCtxtPtr ctxt, xmlDocPtr doc, xmlNodePtr tree) { if (xmlXIncludeTestNode(ctxt, cur) == 1) { xmlXIncludePreProcessNode(ctxt, cur); } else if ((cur->children != NULL) && - (cur->children->type != XML_ENTITY_DECL) && - (cur->children->type != XML_XINCLUDE_START) && - (cur->children->type != XML_XINCLUDE_END)) { + ((cur->type == XML_DOCUMENT_NODE) || + (cur->type == XML_ELEMENT_NODE))) { cur = cur->children; continue; } -- 2.31.1