diff options
author | Kai Kang <kai.kang@windriver.com> | 2022-03-31 13:51:37 +0800 |
---|---|---|
committer | Anuj Mittal <anuj.mittal@intel.com> | 2022-04-13 12:40:51 +0800 |
commit | c931d79ccb294322fb738aaadc41df6960df96e7 (patch) | |
tree | dd13a7636d011d50e6a9757178bbeb99502f089b /meta | |
parent | fbbb689c8df7f82644b8a9bc5bb6884bc6516660 (diff) | |
download | openembedded-core-contrib-c931d79ccb294322fb738aaadc41df6960df96e7.tar.gz |
expat: fix CVE-2022-25313
Backport patch to fix CVE-2022-25313.
CVE: CVE-2022-25313
Signed-off-by: Kai Kang <kai.kang@windriver.com>
Signed-off-by: Anuj Mittal <anuj.mittal@intel.com>
Diffstat (limited to 'meta')
-rw-r--r-- | meta/recipes-core/expat/expat/CVE-2022-25313.patch | 233 | ||||
-rw-r--r-- | meta/recipes-core/expat/expat_2.2.10.bb | 1 |
2 files changed, 234 insertions, 0 deletions
diff --git a/meta/recipes-core/expat/expat/CVE-2022-25313.patch b/meta/recipes-core/expat/expat/CVE-2022-25313.patch new file mode 100644 index 0000000000..569324303e --- /dev/null +++ b/meta/recipes-core/expat/expat/CVE-2022-25313.patch @@ -0,0 +1,233 @@ +CVE: CVE-2022-25313 +Upstream-Status: Backport [https://github.com/libexpat/libexpat/commit/9b4ce651] + +Ref: +* https://github.com/libexpat/libexpat/pull/558 + +Signed-off-by: Kai Kang <kai.kang@windriver.com> + +From 9b4ce651b26557f16103c3a366c91934ecd439ab Mon Sep 17 00:00:00 2001 +From: Samanta Navarro <ferivoz@riseup.net> +Date: Tue, 15 Feb 2022 11:54:29 +0000 +Subject: [PATCH] Prevent stack exhaustion in build_model + +It is possible to trigger stack exhaustion in build_model function if +depth of nested children in DTD element is large enough. This happens +because build_node is a recursively called function within build_model. + +The code has been adjusted to run iteratively. It uses the already +allocated heap space as temporary stack (growing from top to bottom). + +Output is identical to recursive version. No new fields in data +structures were added, i.e. it keeps full API and ABI compatibility. +Instead the numchildren variable is used to temporarily keep the +index of items (uint vs int). + +Documentation and readability improvements kindly added by Sebastian. + +Proof of Concept: + +1. Compile poc binary which parses XML file line by line + +``` +cat > poc.c << EOF + #include <err.h> + #include <expat.h> + #include <stdio.h> + + XML_Parser parser; + + static void XMLCALL + dummy_element_decl_handler(void *userData, const XML_Char *name, + XML_Content *model) { + XML_FreeContentModel(parser, model); + } + + int main(int argc, char *argv[]) { + FILE *fp; + char *p = NULL; + size_t s = 0; + ssize_t l; + if (argc != 2) + errx(1, "usage: poc poc.xml"); + if ((parser = XML_ParserCreate(NULL)) == NULL) + errx(1, "XML_ParserCreate"); + XML_SetElementDeclHandler(parser, dummy_element_decl_handler); + if ((fp = fopen(argv[1], "r")) == NULL) + err(1, "fopen"); + while ((l = getline(&p, &s, fp)) > 0) + if (XML_Parse(parser, p, (int)l, XML_FALSE) != XML_STATUS_OK) + errx(1, "XML_Parse"); + XML_ParserFree(parser); + free(p); + fclose(fp); + return 0; + } +EOF +cc -std=c11 -D_POSIX_C_SOURCE=200809L -lexpat -o poc poc.c +``` + +2. Create XML file with a lot of nested groups in DTD element + +``` +cat > poc.xml.zst.b64 << EOF +KLUv/aQkACAAPAEA+DwhRE9DVFlQRSB1d3UgWwo8IUVMRU1FTlQgdXd1CigBAHv/58AJAgAQKAIA +ECgCABAoAgAQKAIAECgCABAoAgAQKHwAAChvd28KKQIA2/8gV24XBAIAECkCABApAgAQKQIAECkC +ABApAgAQKQIAEClVAAAgPl0+CgEA4A4I2VwwnQ== +EOF +base64 -d poc.xml.zst.b64 | zstd -d > poc.xml +``` + +3. Run Proof of Concept + +``` +./poc poc.xml +``` + +Co-authored-by: Sebastian Pipping <sebastian@pipping.org> +--- + expat/lib/xmlparse.c | 116 +++++++++++++++++++++++++++++-------------- + 1 file changed, 79 insertions(+), 37 deletions(-) + +diff --git a/expat/lib/xmlparse.c b/expat/lib/xmlparse.c +index 4b43e613..594cf12c 100644 +--- a/lib/xmlparse.c ++++ b/lib/xmlparse.c +@@ -7317,44 +7317,15 @@ nextScaffoldPart(XML_Parser parser) { + return next; + } + +-static void +-build_node(XML_Parser parser, int src_node, XML_Content *dest, +- XML_Content **contpos, XML_Char **strpos) { +- DTD *const dtd = parser->m_dtd; /* save one level of indirection */ +- dest->type = dtd->scaffold[src_node].type; +- dest->quant = dtd->scaffold[src_node].quant; +- if (dest->type == XML_CTYPE_NAME) { +- const XML_Char *src; +- dest->name = *strpos; +- src = dtd->scaffold[src_node].name; +- for (;;) { +- *(*strpos)++ = *src; +- if (! *src) +- break; +- src++; +- } +- dest->numchildren = 0; +- dest->children = NULL; +- } else { +- unsigned int i; +- int cn; +- dest->numchildren = dtd->scaffold[src_node].childcnt; +- dest->children = *contpos; +- *contpos += dest->numchildren; +- for (i = 0, cn = dtd->scaffold[src_node].firstchild; i < dest->numchildren; +- i++, cn = dtd->scaffold[cn].nextsib) { +- build_node(parser, cn, &(dest->children[i]), contpos, strpos); +- } +- dest->name = NULL; +- } +-} +- + static XML_Content * + build_model(XML_Parser parser) { ++ /* Function build_model transforms the existing parser->m_dtd->scaffold ++ * array of CONTENT_SCAFFOLD tree nodes into a new array of ++ * XML_Content tree nodes followed by a gapless list of zero-terminated ++ * strings. */ + DTD *const dtd = parser->m_dtd; /* save one level of indirection */ + XML_Content *ret; +- XML_Content *cpos; +- XML_Char *str; ++ XML_Char *str; /* the current string writing location */ + + /* Detect and prevent integer overflow. + * The preprocessor guard addresses the "always false" warning +@@ -7380,10 +7351,81 @@ build_model(XML_Parser parser) { + if (! ret) + return NULL; + +- str = (XML_Char *)(&ret[dtd->scaffCount]); +- cpos = &ret[1]; ++ /* What follows is an iterative implementation (of what was previously done ++ * recursively in a dedicated function called "build_node". The old recursive ++ * build_node could be forced into stack exhaustion from input as small as a ++ * few megabyte, and so that was a security issue. Hence, a function call ++ * stack is avoided now by resolving recursion.) ++ * ++ * The iterative approach works as follows: ++ * ++ * - We use space in the target array for building a temporary stack structure ++ * while that space is still unused. ++ * The stack grows from the array's end downwards and the "actual data" ++ * grows from the start upwards, sequentially. ++ * (Because stack grows downwards, pushing onto the stack is a decrement ++ * while popping off the stack is an increment.) ++ * ++ * - A stack element appears as a regular XML_Content node on the outside, ++ * but only uses a single field -- numchildren -- to store the source ++ * tree node array index. These are the breadcrumbs leading the way back ++ * during pre-order (node first) depth-first traversal. ++ * ++ * - The reason we know the stack will never grow into (or overlap with) ++ * the area with data of value at the start of the array is because ++ * the overall number of elements to process matches the size of the array, ++ * and the sum of fully processed nodes and yet-to-be processed nodes ++ * on the stack, cannot be more than the total number of nodes. ++ * It is possible for the top of the stack and the about-to-write node ++ * to meet, but that is safe because we get the source index out ++ * before doing any writes on that node. ++ */ ++ XML_Content *dest = ret; /* tree node writing location, moves upwards */ ++ XML_Content *const destLimit = &ret[dtd->scaffCount]; ++ XML_Content *const stackBottom = &ret[dtd->scaffCount]; ++ XML_Content *stackTop = stackBottom; /* i.e. stack is initially empty */ ++ str = (XML_Char *)&ret[dtd->scaffCount]; ++ ++ /* Push source tree root node index onto the stack */ ++ (--stackTop)->numchildren = 0; ++ ++ for (; dest < destLimit; dest++) { ++ /* Pop source tree node index off the stack */ ++ const int src_node = (int)(stackTop++)->numchildren; ++ ++ /* Convert item */ ++ dest->type = dtd->scaffold[src_node].type; ++ dest->quant = dtd->scaffold[src_node].quant; ++ if (dest->type == XML_CTYPE_NAME) { ++ const XML_Char *src; ++ dest->name = str; ++ src = dtd->scaffold[src_node].name; ++ for (;;) { ++ *str++ = *src; ++ if (! *src) ++ break; ++ src++; ++ } ++ dest->numchildren = 0; ++ dest->children = NULL; ++ } else { ++ unsigned int i; ++ int cn; ++ dest->name = NULL; ++ dest->numchildren = dtd->scaffold[src_node].childcnt; ++ dest->children = &dest[1]; ++ ++ /* Push children to the stack ++ * in a way where the first child ends up at the top of the ++ * (downwards growing) stack, in order to be processed first. */ ++ stackTop -= dest->numchildren; ++ for (i = 0, cn = dtd->scaffold[src_node].firstchild; ++ i < dest->numchildren; i++, cn = dtd->scaffold[cn].nextsib) { ++ (stackTop + i)->numchildren = (unsigned int)cn; ++ } ++ } ++ } + +- build_node(parser, 0, ret, &cpos, &str); + return ret; + } + +-- +2.33.0 + diff --git a/meta/recipes-core/expat/expat_2.2.10.bb b/meta/recipes-core/expat/expat_2.2.10.bb index f99fa7edb6..7454718dca 100644 --- a/meta/recipes-core/expat/expat_2.2.10.bb +++ b/meta/recipes-core/expat/expat_2.2.10.bb @@ -20,6 +20,7 @@ SRC_URI = "https://github.com/libexpat/libexpat/releases/download/R_${VERSION_TA file://CVE-2022-25235.patch \ file://CVE-2022-25236-1.patch \ file://CVE-2022-25236-2.patch \ + file://CVE-2022-25313.patch \ " UPSTREAM_CHECK_URI = "https://github.com/libexpat/libexpat/releases/" |