cheng/libexpat
1
0
Fork 0
Code Issues Pull Requests Actions 8 Packages Projects Releases Wiki Activity

Merge pull request #566 from ferivoz/model-regression

Browse Source 
Fix build_model regression
This commit is contained in:
Sebastian Pipping 2022-02-20 15:57:54 +01:00 committed by GitHub
commit 9288cd5474
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 140 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
expat/Changes
View File

@ -2,6 +2,22 @@ NOTE: We are looking for help with a few things:
https://github.com/libexpat/libexpat/labels/help%20wanted
If you can help, please get in touch. Thanks!
Release x.x.x xxx xxxxxxxx xx xxxx
Bug fixes:
#??? Fix a regression intruced by the fix for CVE-2022-25313
in release 2.4.5 that affects applications that (1)
call function XML_SetElementDeclHandler and (2) are
parsing XML that contains nested element declarations
(e.g. "<!ELEMENT junk ((bar|foo|xyz+), zebra*)>").
Special thanks to:
Matt Sergeant
Samanta Navarro
Sergei Trofimovich
and
NixOS
Perl XML::Parser
Release 2.4.5 Fri February 18 2022
Security fixes:
#562 CVE-2022-25235 -- Passing malformed 2- and 3-byte UTF-8

79
expat/lib/xmlparse.c
View File

@ -7373,39 +7373,58 @@ build_model(XML_Parser parser) {
*
* 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.)
* - We have two writing pointers, both walking up the result array; one does
* the work, the other creates "jobs" for its colleague to do, and leads
* the way:
*
* - 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 faster one, pointer jobDest, always leads and writes "what job
* to do" by the other, once they reach that place in the
* array: leader "jobDest" stores the source node array index (relative
* to array dtd->scaffold) in field "numchildren".
*
* - 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.
* - The slower one, pointer dest, looks at the value stored in the
* "numchildren" field (which actually holds a source node array index
* at that time) and puts the real data from dtd->scaffold in.
*
* - Before the loop starts, jobDest writes source array index 0
* (where the root node is located) so that dest will have something to do
* when it starts operation.
*
* - Whenever nodes with children are encountered, jobDest appends
* them as new jobs, in order. As a result, tree node siblings are
* adjacent in the resulting array, for example:
*
* [0] root, has two children
* [1] first child of 0, has three children
* [3] first child of 1, does not have children
* [4] second child of 1, does not have children
* [5] third child of 1, does not have children
* [2] second child of 0, does not have children
*
* Or (the same data) presented in flat array view:
*
* [0] root, has two children
*
* [1] first child of 0, has three children
* [2] second child of 0, does not have children
*
* [3] first child of 1, does not have children
* [4] second child of 1, does not have children
* [5] third child of 1, does not have children
*
* - The algorithm repeats until all target array indices have been processed.
*/
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 */
XML_Content *jobDest = ret; /* next free writing location in target array */
str = (XML_Char *)&ret[dtd->scaffCount];
/* Push source tree root node index onto the stack */
(--stackTop)->numchildren = 0;
/* Add the starting job, the root node (index 0) of the source tree */
(jobDest++)->numchildren = 0;
for (; dest < destLimit; dest++) {
/* Pop source tree node index off the stack */
const int src_node = (int)(stackTop++)->numchildren;
/* Retrieve source tree array index from job storage */
const int src_node = (int)dest->numchildren;
/* Convert item */
dest->type = dtd->scaffold[src_node].type;
@ -7427,16 +7446,12 @@ build_model(XML_Parser parser) {
int cn;
dest->name = NULL;
dest->numchildren = dtd->scaffold[src_node].childcnt;
dest->children = &dest[1];
dest->children = jobDest;
/* 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;
/* Append scaffold indices of children to array */
for (i = 0, cn = dtd->scaffold[src_node].firstchild;
i < dest->numchildren; i++, cn = dtd->scaffold[cn].nextsib) {
(stackTop + i)->numchildren = (unsigned int)cn;
}
i < dest->numchildren; i++, cn = dtd->scaffold[cn].nextsib)
(jobDest++)->numchildren = (unsigned int)cn;
}
}

77
expat/tests/runtests.c
View File

@ -2664,6 +2664,82 @@ START_TEST(test_dtd_elements) {
}
END_TEST
static void XMLCALL
element_decl_check_model(void *userData, const XML_Char *name,
XML_Content *model) {
UNUSED_P(userData);
uint32_t errorFlags = 0;
/* Expected model array structure is this:
* [0] (type 6, quant 0)
* [1] (type 5, quant 0)
* [3] (type 4, quant 0, name "bar")
* [4] (type 4, quant 0, name "foo")
* [5] (type 4, quant 3, name "xyz")
* [2] (type 4, quant 2, name "zebra")
*/
errorFlags |= ((xcstrcmp(name, XCS("junk")) == 0) ? 0 : (1u << 0));
errorFlags |= ((model != NULL) ? 0 : (1u << 1));
errorFlags |= ((model[0].type == XML_CTYPE_SEQ) ? 0 : (1u << 2));
errorFlags |= ((model[0].quant == XML_CQUANT_NONE) ? 0 : (1u << 3));
errorFlags |= ((model[0].numchildren == 2) ? 0 : (1u << 4));
errorFlags |= ((model[0].children == &model[1]) ? 0 : (1u << 5));
errorFlags |= ((model[0].name == NULL) ? 0 : (1u << 6));
errorFlags |= ((model[1].type == XML_CTYPE_CHOICE) ? 0 : (1u << 7));
errorFlags |= ((model[1].quant == XML_CQUANT_NONE) ? 0 : (1u << 8));
errorFlags |= ((model[1].numchildren == 3) ? 0 : (1u << 9));
errorFlags |= ((model[1].children == &model[3]) ? 0 : (1u << 10));
errorFlags |= ((model[1].name == NULL) ? 0 : (1u << 11));
errorFlags |= ((model[2].type == XML_CTYPE_NAME) ? 0 : (1u << 12));
errorFlags |= ((model[2].quant == XML_CQUANT_REP) ? 0 : (1u << 13));
errorFlags |= ((model[2].numchildren == 0) ? 0 : (1u << 14));
errorFlags |= ((model[2].children == NULL) ? 0 : (1u << 15));
errorFlags |= ((xcstrcmp(model[2].name, XCS("zebra")) == 0) ? 0 : (1u << 16));
errorFlags |= ((model[3].type == XML_CTYPE_NAME) ? 0 : (1u << 17));
errorFlags |= ((model[3].quant == XML_CQUANT_NONE) ? 0 : (1u << 18));
errorFlags |= ((model[3].numchildren == 0) ? 0 : (1u << 19));
errorFlags |= ((model[3].children == NULL) ? 0 : (1u << 20));
errorFlags |= ((xcstrcmp(model[3].name, XCS("bar")) == 0) ? 0 : (1u << 21));
errorFlags |= ((model[4].type == XML_CTYPE_NAME) ? 0 : (1u << 22));
errorFlags |= ((model[4].quant == XML_CQUANT_NONE) ? 0 : (1u << 23));
errorFlags |= ((model[4].numchildren == 0) ? 0 : (1u << 24));
errorFlags |= ((model[4].children == NULL) ? 0 : (1u << 25));
errorFlags |= ((xcstrcmp(model[4].name, XCS("foo")) == 0) ? 0 : (1u << 26));
errorFlags |= ((model[5].type == XML_CTYPE_NAME) ? 0 : (1u << 27));
errorFlags |= ((model[5].quant == XML_CQUANT_PLUS) ? 0 : (1u << 28));
errorFlags |= ((model[5].numchildren == 0) ? 0 : (1u << 29));
errorFlags |= ((model[5].children == NULL) ? 0 : (1u << 30));
errorFlags |= ((xcstrcmp(model[5].name, XCS("xyz")) == 0) ? 0 : (1u << 31));
XML_SetUserData(g_parser, (void *)(uintptr_t)errorFlags);
XML_FreeContentModel(g_parser, model);
}
START_TEST(test_dtd_elements_nesting) {
// Payload inspired by a test in Perl's XML::Parser
const char *text = "<!DOCTYPE foo [\n"
"<!ELEMENT junk ((bar|foo|xyz+), zebra*)>\n"
"]>\n"
"<foo/>";
XML_SetUserData(g_parser, (void *)(uintptr_t)-1);
XML_SetElementDeclHandler(g_parser, element_decl_check_model);
if (XML_Parse(g_parser, text, (int)strlen(text), XML_TRUE)
== XML_STATUS_ERROR)
xml_failure(g_parser);
if ((uint32_t)(uintptr_t)XML_GetUserData(g_parser) != 0)
fail("Element declaration model regression detected");
}
END_TEST
/* Test foreign DTD handling */
START_TEST(test_set_foreign_dtd) {
const char *text1 = "<?xml version='1.0' encoding='us-ascii'?>\n";
@ -11863,6 +11939,7 @@ make_suite(void) {
tcase_add_test(tc_basic, test_memory_allocation);
tcase_add_test(tc_basic, test_default_current);
tcase_add_test(tc_basic, test_dtd_elements);
tcase_add_test(tc_basic, test_dtd_elements_nesting);
tcase_add_test__ifdef_xml_dtd(tc_basic, test_set_foreign_dtd);
tcase_add_test__ifdef_xml_dtd(tc_basic, test_foreign_dtd_not_standalone);
tcase_add_test__ifdef_xml_dtd(tc_basic, test_invalid_foreign_dtd);