Re: [oe-core][dunfell][PATCH v2] libxml2: Fix CVE-2021-3518

2021-06-22 Thread Steve Sakoman
On Mon, Jun 21, 2021 at 5:34 AM Jasper Orschulko via
lists.openembedded.org 
wrote:
>
> There's a flaw in libxml2 in versions before 2.9.11. An attacker who is able 
> to submit a crafted file to be processed by an application linked with 
> libxml2 could trigger a use-after-free. The greatest impact from this flaw is 
> to confidentiality, integrity, and availability.
>
> Upstream-Status: Backport [from fedora:
> https://bugzilla.redhat.com/show_bug.cgi?id=1954243]
>
> Signed-off-by: Jasper Orschulko 
> ---
>  .../libxml/libxml2/CVE-2021-3518.patch| 108 ++
>  meta/recipes-core/libxml/libxml2_2.9.10.bb|   1 +
>  2 files changed, 109 insertions(+)
>  create mode 100644 meta/recipes-core/libxml/libxml2/CVE-2021-3518.patch
>
> diff --git a/meta/recipes-core/libxml/libxml2/CVE-2021-3518.patch 
> b/meta/recipes-core/libxml/libxml2/CVE-2021-3518.patch
> new file mode 100644
> index 00..c22cccf1b1
> --- /dev/null
> +++ b/meta/recipes-core/libxml/libxml2/CVE-2021-3518.patch
> @@ -0,0 +1,108 @@
> +From ac82a514e16eb81b4506e2cba1a1ee45b9f025b5 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.
> +
> +The upstream patch 752e5f71d7cea2ca5a7e7c0b8f72ed04ce654be4 has been 
> modified,
> +as to avoid unnecessary modifications to fallback files.
> +
> +Signed-off-by: Jasper Orschulko 

I'm sorry I didn't notice this on the first review, but this patch is
also missing the CVE and Upstream-Status tags.

Thanks,

Steve

> +---
> + xinclude.c | 24 ++--
> + 1 file changed, 10 insertions(+), 14 deletions(-)
> +
> +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.32.0
> +
> +
> +From 3ad5ac1e39e3cd42f838c1cd27ffd4e9b79e6121 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.
> +
> +Signed-off-by: Jasper Orschulko 
> +---
> + 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) {
> +

[oe-core][dunfell][PATCH v2] libxml2: Fix CVE-2021-3518

2021-06-21 Thread Jasper Orschulko via lists.openembedded.org
There's a flaw in libxml2 in versions before 2.9.11. An attacker who is able to 
submit a crafted file to be processed by an application linked with libxml2 
could trigger a use-after-free. The greatest impact from this flaw is to 
confidentiality, integrity, and availability.

Upstream-Status: Backport [from fedora:
https://bugzilla.redhat.com/show_bug.cgi?id=1954243]

Signed-off-by: Jasper Orschulko 
---
 .../libxml/libxml2/CVE-2021-3518.patch| 108 ++
 meta/recipes-core/libxml/libxml2_2.9.10.bb|   1 +
 2 files changed, 109 insertions(+)
 create mode 100644 meta/recipes-core/libxml/libxml2/CVE-2021-3518.patch

diff --git a/meta/recipes-core/libxml/libxml2/CVE-2021-3518.patch 
b/meta/recipes-core/libxml/libxml2/CVE-2021-3518.patch
new file mode 100644
index 00..c22cccf1b1
--- /dev/null
+++ b/meta/recipes-core/libxml/libxml2/CVE-2021-3518.patch
@@ -0,0 +1,108 @@
+From ac82a514e16eb81b4506e2cba1a1ee45b9f025b5 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.
+
+The upstream patch 752e5f71d7cea2ca5a7e7c0b8f72ed04ce654be4 has been modified,
+as to avoid unnecessary modifications to fallback files.
+
+Signed-off-by: Jasper Orschulko 
+---
+ xinclude.c | 24 ++--
+ 1 file changed, 10 insertions(+), 14 deletions(-)
+
+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.32.0
+
+
+From 3ad5ac1e39e3cd42f838c1cd27ffd4e9b79e6121 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.
+
+Signed-off-by: Jasper Orschulko 
+---
+ 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;