diff options
author | Steve Sakoman <steve@sakoman.com> | 2022-02-28 05:43:58 -1000 |
---|---|---|
committer | Richard Purdie <richard.purdie@linuxfoundation.org> | 2022-03-09 17:30:48 +0000 |
commit | e173db21d0899a5cb185f01f8aaf92156d3e910c (patch) | |
tree | 1698cef9239d34c763761f62a30495148fd1643f /meta | |
parent | 746111afa001dc99c95fc56dc242b5f00a0bc1b9 (diff) | |
download | poky-e173db21d0899a5cb185f01f8aaf92156d3e910c.tar.gz |
expat: fix CVE-2022-25313
In Expat (aka libexpat) before 2.4.5, an attacker can trigger stack
exhaustion in build_model via a large nesting depth in the DTD element.
Backport patch from:
https://github.com/libexpat/libexpat/pull/558/commits/9b4ce651b26557f16103c3a366c91934ecd439ab
Also add patch which fixes a regression introduced in the above fix:
https://github.com/libexpat/libexpat/pull/566
CVE: CVE-2022-25313
(From OE-Core rev: 8105700b1d6d23c87332f453bdc7379999bb4b03)
Signed-off-by: Steve Sakoman <steve@sakoman.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
Diffstat (limited to 'meta')
-rw-r--r-- | meta/recipes-core/expat/expat/CVE-2022-25313-regression.patch | 131 | ||||
-rw-r--r-- | meta/recipes-core/expat/expat/CVE-2022-25313.patch | 230 | ||||
-rw-r--r-- | meta/recipes-core/expat/expat_2.2.9.bb | 2 |
3 files changed, 363 insertions, 0 deletions
diff --git a/meta/recipes-core/expat/expat/CVE-2022-25313-regression.patch b/meta/recipes-core/expat/expat/CVE-2022-25313-regression.patch new file mode 100644 index 0000000000..af255e8cb5 --- /dev/null +++ b/meta/recipes-core/expat/expat/CVE-2022-25313-regression.patch | |||
@@ -0,0 +1,131 @@ | |||
1 | From b12f34fe32821a69dc12ff9a021daca0856de238 Mon Sep 17 00:00:00 2001 | ||
2 | From: Samanta Navarro <ferivoz@riseup.net> | ||
3 | Date: Sat, 19 Feb 2022 23:59:25 +0000 | ||
4 | Subject: [PATCH] Fix build_model regression. | ||
5 | |||
6 | The iterative approach in build_model failed to fill children arrays | ||
7 | correctly. A preorder traversal is not required and turned out to be the | ||
8 | culprit. Use an easier algorithm: | ||
9 | |||
10 | Add nodes from scaffold tree starting at index 0 (root) to the target | ||
11 | array whenever children are encountered. This ensures that children | ||
12 | are adjacent to each other. This complies with the recursive version. | ||
13 | |||
14 | Store only the scaffold index in numchildren field to prevent a direct | ||
15 | processing of these children, which would require a recursive solution. | ||
16 | This allows the algorithm to iterate through the target array from start | ||
17 | to end without jumping back and forth, converting on the fly. | ||
18 | |||
19 | Co-authored-by: Sebastian Pipping <sebastian@pipping.org> | ||
20 | --- | ||
21 | lib/xmlparse.c | 79 ++++++++++++++++++++++++++------------------ | ||
22 | 1 file changed, 47 insertions(+), 32 deletions(-) | ||
23 | |||
24 | diff --git a/lib/xmlparse.c b/lib/xmlparse.c | ||
25 | index c479a258..84885b5a 100644 | ||
26 | --- a/lib/xmlparse.c | ||
27 | +++ b/lib/xmlparse.c | ||
28 | @@ -7373,39 +7373,58 @@ build_model(XML_Parser parser) { | ||
29 | * | ||
30 | * The iterative approach works as follows: | ||
31 | * | ||
32 | - * - We use space in the target array for building a temporary stack structure | ||
33 | - * while that space is still unused. | ||
34 | - * The stack grows from the array's end downwards and the "actual data" | ||
35 | - * grows from the start upwards, sequentially. | ||
36 | - * (Because stack grows downwards, pushing onto the stack is a decrement | ||
37 | - * while popping off the stack is an increment.) | ||
38 | + * - We have two writing pointers, both walking up the result array; one does | ||
39 | + * the work, the other creates "jobs" for its colleague to do, and leads | ||
40 | + * the way: | ||
41 | * | ||
42 | - * - A stack element appears as a regular XML_Content node on the outside, | ||
43 | - * but only uses a single field -- numchildren -- to store the source | ||
44 | - * tree node array index. These are the breadcrumbs leading the way back | ||
45 | - * during pre-order (node first) depth-first traversal. | ||
46 | + * - The faster one, pointer jobDest, always leads and writes "what job | ||
47 | + * to do" by the other, once they reach that place in the | ||
48 | + * array: leader "jobDest" stores the source node array index (relative | ||
49 | + * to array dtd->scaffold) in field "numchildren". | ||
50 | * | ||
51 | - * - The reason we know the stack will never grow into (or overlap with) | ||
52 | - * the area with data of value at the start of the array is because | ||
53 | - * the overall number of elements to process matches the size of the array, | ||
54 | - * and the sum of fully processed nodes and yet-to-be processed nodes | ||
55 | - * on the stack, cannot be more than the total number of nodes. | ||
56 | - * It is possible for the top of the stack and the about-to-write node | ||
57 | - * to meet, but that is safe because we get the source index out | ||
58 | - * before doing any writes on that node. | ||
59 | + * - The slower one, pointer dest, looks at the value stored in the | ||
60 | + * "numchildren" field (which actually holds a source node array index | ||
61 | + * at that time) and puts the real data from dtd->scaffold in. | ||
62 | + * | ||
63 | + * - Before the loop starts, jobDest writes source array index 0 | ||
64 | + * (where the root node is located) so that dest will have something to do | ||
65 | + * when it starts operation. | ||
66 | + * | ||
67 | + * - Whenever nodes with children are encountered, jobDest appends | ||
68 | + * them as new jobs, in order. As a result, tree node siblings are | ||
69 | + * adjacent in the resulting array, for example: | ||
70 | + * | ||
71 | + * [0] root, has two children | ||
72 | + * [1] first child of 0, has three children | ||
73 | + * [3] first child of 1, does not have children | ||
74 | + * [4] second child of 1, does not have children | ||
75 | + * [5] third child of 1, does not have children | ||
76 | + * [2] second child of 0, does not have children | ||
77 | + * | ||
78 | + * Or (the same data) presented in flat array view: | ||
79 | + * | ||
80 | + * [0] root, has two children | ||
81 | + * | ||
82 | + * [1] first child of 0, has three children | ||
83 | + * [2] second child of 0, does not have children | ||
84 | + * | ||
85 | + * [3] first child of 1, does not have children | ||
86 | + * [4] second child of 1, does not have children | ||
87 | + * [5] third child of 1, does not have children | ||
88 | + * | ||
89 | + * - The algorithm repeats until all target array indices have been processed. | ||
90 | */ | ||
91 | XML_Content *dest = ret; /* tree node writing location, moves upwards */ | ||
92 | XML_Content *const destLimit = &ret[dtd->scaffCount]; | ||
93 | - XML_Content *const stackBottom = &ret[dtd->scaffCount]; | ||
94 | - XML_Content *stackTop = stackBottom; /* i.e. stack is initially empty */ | ||
95 | + XML_Content *jobDest = ret; /* next free writing location in target array */ | ||
96 | str = (XML_Char *)&ret[dtd->scaffCount]; | ||
97 | |||
98 | - /* Push source tree root node index onto the stack */ | ||
99 | - (--stackTop)->numchildren = 0; | ||
100 | + /* Add the starting job, the root node (index 0) of the source tree */ | ||
101 | + (jobDest++)->numchildren = 0; | ||
102 | |||
103 | for (; dest < destLimit; dest++) { | ||
104 | - /* Pop source tree node index off the stack */ | ||
105 | - const int src_node = (int)(stackTop++)->numchildren; | ||
106 | + /* Retrieve source tree array index from job storage */ | ||
107 | + const int src_node = (int)dest->numchildren; | ||
108 | |||
109 | /* Convert item */ | ||
110 | dest->type = dtd->scaffold[src_node].type; | ||
111 | @@ -7427,16 +7446,12 @@ build_model(XML_Parser parser) { | ||
112 | int cn; | ||
113 | dest->name = NULL; | ||
114 | dest->numchildren = dtd->scaffold[src_node].childcnt; | ||
115 | - dest->children = &dest[1]; | ||
116 | + dest->children = jobDest; | ||
117 | |||
118 | - /* Push children to the stack | ||
119 | - * in a way where the first child ends up at the top of the | ||
120 | - * (downwards growing) stack, in order to be processed first. */ | ||
121 | - stackTop -= dest->numchildren; | ||
122 | + /* Append scaffold indices of children to array */ | ||
123 | for (i = 0, cn = dtd->scaffold[src_node].firstchild; | ||
124 | - i < dest->numchildren; i++, cn = dtd->scaffold[cn].nextsib) { | ||
125 | - (stackTop + i)->numchildren = (unsigned int)cn; | ||
126 | - } | ||
127 | + i < dest->numchildren; i++, cn = dtd->scaffold[cn].nextsib) | ||
128 | + (jobDest++)->numchildren = (unsigned int)cn; | ||
129 | } | ||
130 | } | ||
131 | |||
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..470d66e9dd --- /dev/null +++ b/meta/recipes-core/expat/expat/CVE-2022-25313.patch | |||
@@ -0,0 +1,230 @@ | |||
1 | From 9b4ce651b26557f16103c3a366c91934ecd439ab Mon Sep 17 00:00:00 2001 | ||
2 | From: Samanta Navarro <ferivoz@riseup.net> | ||
3 | Date: Tue, 15 Feb 2022 11:54:29 +0000 | ||
4 | Subject: [PATCH] Prevent stack exhaustion in build_model | ||
5 | |||
6 | It is possible to trigger stack exhaustion in build_model function if | ||
7 | depth of nested children in DTD element is large enough. This happens | ||
8 | because build_node is a recursively called function within build_model. | ||
9 | |||
10 | The code has been adjusted to run iteratively. It uses the already | ||
11 | allocated heap space as temporary stack (growing from top to bottom). | ||
12 | |||
13 | Output is identical to recursive version. No new fields in data | ||
14 | structures were added, i.e. it keeps full API and ABI compatibility. | ||
15 | Instead the numchildren variable is used to temporarily keep the | ||
16 | index of items (uint vs int). | ||
17 | |||
18 | Documentation and readability improvements kindly added by Sebastian. | ||
19 | |||
20 | Proof of Concept: | ||
21 | |||
22 | 1. Compile poc binary which parses XML file line by line | ||
23 | |||
24 | ``` | ||
25 | cat > poc.c << EOF | ||
26 | #include <err.h> | ||
27 | #include <expat.h> | ||
28 | #include <stdio.h> | ||
29 | |||
30 | XML_Parser parser; | ||
31 | |||
32 | static void XMLCALL | ||
33 | dummy_element_decl_handler(void *userData, const XML_Char *name, | ||
34 | XML_Content *model) { | ||
35 | XML_FreeContentModel(parser, model); | ||
36 | } | ||
37 | |||
38 | int main(int argc, char *argv[]) { | ||
39 | FILE *fp; | ||
40 | char *p = NULL; | ||
41 | size_t s = 0; | ||
42 | ssize_t l; | ||
43 | if (argc != 2) | ||
44 | errx(1, "usage: poc poc.xml"); | ||
45 | if ((parser = XML_ParserCreate(NULL)) == NULL) | ||
46 | errx(1, "XML_ParserCreate"); | ||
47 | XML_SetElementDeclHandler(parser, dummy_element_decl_handler); | ||
48 | if ((fp = fopen(argv[1], "r")) == NULL) | ||
49 | err(1, "fopen"); | ||
50 | while ((l = getline(&p, &s, fp)) > 0) | ||
51 | if (XML_Parse(parser, p, (int)l, XML_FALSE) != XML_STATUS_OK) | ||
52 | errx(1, "XML_Parse"); | ||
53 | XML_ParserFree(parser); | ||
54 | free(p); | ||
55 | fclose(fp); | ||
56 | return 0; | ||
57 | } | ||
58 | EOF | ||
59 | cc -std=c11 -D_POSIX_C_SOURCE=200809L -lexpat -o poc poc.c | ||
60 | ``` | ||
61 | |||
62 | 2. Create XML file with a lot of nested groups in DTD element | ||
63 | |||
64 | ``` | ||
65 | cat > poc.xml.zst.b64 << EOF | ||
66 | KLUv/aQkACAAPAEA+DwhRE9DVFlQRSB1d3UgWwo8IUVMRU1FTlQgdXd1CigBAHv/58AJAgAQKAIA | ||
67 | ECgCABAoAgAQKAIAECgCABAoAgAQKHwAAChvd28KKQIA2/8gV24XBAIAECkCABApAgAQKQIAECkC | ||
68 | ABApAgAQKQIAEClVAAAgPl0+CgEA4A4I2VwwnQ== | ||
69 | EOF | ||
70 | base64 -d poc.xml.zst.b64 | zstd -d > poc.xml | ||
71 | ``` | ||
72 | |||
73 | 3. Run Proof of Concept | ||
74 | |||
75 | ``` | ||
76 | ./poc poc.xml | ||
77 | ``` | ||
78 | |||
79 | Co-authored-by: Sebastian Pipping <sebastian@pipping.org> | ||
80 | |||
81 | Upstream-Status: Backport | ||
82 | https://github.com/libexpat/libexpat/pull/558/commits/9b4ce651b26557f16103c3a366c91934ecd439ab | ||
83 | |||
84 | CVE: CVE-2022-25313 | ||
85 | |||
86 | Signed-off-by: Steve Sakoman <steve@sakoman.com> | ||
87 | |||
88 | --- | ||
89 | expat/lib/xmlparse.c | 116 +++++++++++++++++++++++++++++-------------- | ||
90 | 1 file changed, 79 insertions(+), 37 deletions(-) | ||
91 | |||
92 | diff --git a/lib/xmlparse.c b/lib/xmlparse.c | ||
93 | index 4b43e613..594cf12c 100644 | ||
94 | --- a/lib/xmlparse.c | ||
95 | +++ b/lib/xmlparse.c | ||
96 | @@ -7317,44 +7317,15 @@ nextScaffoldPart(XML_Parser parser) { | ||
97 | return next; | ||
98 | } | ||
99 | |||
100 | -static void | ||
101 | -build_node(XML_Parser parser, int src_node, XML_Content *dest, | ||
102 | - XML_Content **contpos, XML_Char **strpos) { | ||
103 | - DTD *const dtd = parser->m_dtd; /* save one level of indirection */ | ||
104 | - dest->type = dtd->scaffold[src_node].type; | ||
105 | - dest->quant = dtd->scaffold[src_node].quant; | ||
106 | - if (dest->type == XML_CTYPE_NAME) { | ||
107 | - const XML_Char *src; | ||
108 | - dest->name = *strpos; | ||
109 | - src = dtd->scaffold[src_node].name; | ||
110 | - for (;;) { | ||
111 | - *(*strpos)++ = *src; | ||
112 | - if (! *src) | ||
113 | - break; | ||
114 | - src++; | ||
115 | - } | ||
116 | - dest->numchildren = 0; | ||
117 | - dest->children = NULL; | ||
118 | - } else { | ||
119 | - unsigned int i; | ||
120 | - int cn; | ||
121 | - dest->numchildren = dtd->scaffold[src_node].childcnt; | ||
122 | - dest->children = *contpos; | ||
123 | - *contpos += dest->numchildren; | ||
124 | - for (i = 0, cn = dtd->scaffold[src_node].firstchild; i < dest->numchildren; | ||
125 | - i++, cn = dtd->scaffold[cn].nextsib) { | ||
126 | - build_node(parser, cn, &(dest->children[i]), contpos, strpos); | ||
127 | - } | ||
128 | - dest->name = NULL; | ||
129 | - } | ||
130 | -} | ||
131 | - | ||
132 | static XML_Content * | ||
133 | build_model(XML_Parser parser) { | ||
134 | + /* Function build_model transforms the existing parser->m_dtd->scaffold | ||
135 | + * array of CONTENT_SCAFFOLD tree nodes into a new array of | ||
136 | + * XML_Content tree nodes followed by a gapless list of zero-terminated | ||
137 | + * strings. */ | ||
138 | DTD *const dtd = parser->m_dtd; /* save one level of indirection */ | ||
139 | XML_Content *ret; | ||
140 | - XML_Content *cpos; | ||
141 | - XML_Char *str; | ||
142 | + XML_Char *str; /* the current string writing location */ | ||
143 | |||
144 | /* Detect and prevent integer overflow. | ||
145 | * The preprocessor guard addresses the "always false" warning | ||
146 | @@ -7380,10 +7351,81 @@ build_model(XML_Parser parser) { | ||
147 | if (! ret) | ||
148 | return NULL; | ||
149 | |||
150 | - str = (XML_Char *)(&ret[dtd->scaffCount]); | ||
151 | - cpos = &ret[1]; | ||
152 | + /* What follows is an iterative implementation (of what was previously done | ||
153 | + * recursively in a dedicated function called "build_node". The old recursive | ||
154 | + * build_node could be forced into stack exhaustion from input as small as a | ||
155 | + * few megabyte, and so that was a security issue. Hence, a function call | ||
156 | + * stack is avoided now by resolving recursion.) | ||
157 | + * | ||
158 | + * The iterative approach works as follows: | ||
159 | + * | ||
160 | + * - We use space in the target array for building a temporary stack structure | ||
161 | + * while that space is still unused. | ||
162 | + * The stack grows from the array's end downwards and the "actual data" | ||
163 | + * grows from the start upwards, sequentially. | ||
164 | + * (Because stack grows downwards, pushing onto the stack is a decrement | ||
165 | + * while popping off the stack is an increment.) | ||
166 | + * | ||
167 | + * - A stack element appears as a regular XML_Content node on the outside, | ||
168 | + * but only uses a single field -- numchildren -- to store the source | ||
169 | + * tree node array index. These are the breadcrumbs leading the way back | ||
170 | + * during pre-order (node first) depth-first traversal. | ||
171 | + * | ||
172 | + * - The reason we know the stack will never grow into (or overlap with) | ||
173 | + * the area with data of value at the start of the array is because | ||
174 | + * the overall number of elements to process matches the size of the array, | ||
175 | + * and the sum of fully processed nodes and yet-to-be processed nodes | ||
176 | + * on the stack, cannot be more than the total number of nodes. | ||
177 | + * It is possible for the top of the stack and the about-to-write node | ||
178 | + * to meet, but that is safe because we get the source index out | ||
179 | + * before doing any writes on that node. | ||
180 | + */ | ||
181 | + XML_Content *dest = ret; /* tree node writing location, moves upwards */ | ||
182 | + XML_Content *const destLimit = &ret[dtd->scaffCount]; | ||
183 | + XML_Content *const stackBottom = &ret[dtd->scaffCount]; | ||
184 | + XML_Content *stackTop = stackBottom; /* i.e. stack is initially empty */ | ||
185 | + str = (XML_Char *)&ret[dtd->scaffCount]; | ||
186 | + | ||
187 | + /* Push source tree root node index onto the stack */ | ||
188 | + (--stackTop)->numchildren = 0; | ||
189 | + | ||
190 | + for (; dest < destLimit; dest++) { | ||
191 | + /* Pop source tree node index off the stack */ | ||
192 | + const int src_node = (int)(stackTop++)->numchildren; | ||
193 | + | ||
194 | + /* Convert item */ | ||
195 | + dest->type = dtd->scaffold[src_node].type; | ||
196 | + dest->quant = dtd->scaffold[src_node].quant; | ||
197 | + if (dest->type == XML_CTYPE_NAME) { | ||
198 | + const XML_Char *src; | ||
199 | + dest->name = str; | ||
200 | + src = dtd->scaffold[src_node].name; | ||
201 | + for (;;) { | ||
202 | + *str++ = *src; | ||
203 | + if (! *src) | ||
204 | + break; | ||
205 | + src++; | ||
206 | + } | ||
207 | + dest->numchildren = 0; | ||
208 | + dest->children = NULL; | ||
209 | + } else { | ||
210 | + unsigned int i; | ||
211 | + int cn; | ||
212 | + dest->name = NULL; | ||
213 | + dest->numchildren = dtd->scaffold[src_node].childcnt; | ||
214 | + dest->children = &dest[1]; | ||
215 | + | ||
216 | + /* Push children to the stack | ||
217 | + * in a way where the first child ends up at the top of the | ||
218 | + * (downwards growing) stack, in order to be processed first. */ | ||
219 | + stackTop -= dest->numchildren; | ||
220 | + for (i = 0, cn = dtd->scaffold[src_node].firstchild; | ||
221 | + i < dest->numchildren; i++, cn = dtd->scaffold[cn].nextsib) { | ||
222 | + (stackTop + i)->numchildren = (unsigned int)cn; | ||
223 | + } | ||
224 | + } | ||
225 | + } | ||
226 | |||
227 | - build_node(parser, 0, ret, &cpos, &str); | ||
228 | return ret; | ||
229 | } | ||
230 | |||
diff --git a/meta/recipes-core/expat/expat_2.2.9.bb b/meta/recipes-core/expat/expat_2.2.9.bb index c0103767b1..4d945a295e 100644 --- a/meta/recipes-core/expat/expat_2.2.9.bb +++ b/meta/recipes-core/expat/expat_2.2.9.bb | |||
@@ -15,6 +15,8 @@ SRC_URI = "git://github.com/libexpat/libexpat.git;protocol=https;branch=master \ | |||
15 | file://CVE-2022-23990.patch \ | 15 | file://CVE-2022-23990.patch \ |
16 | file://CVE-2022-25235.patch \ | 16 | file://CVE-2022-25235.patch \ |
17 | file://CVE-2022-25236.patch \ | 17 | file://CVE-2022-25236.patch \ |
18 | file://CVE-2022-25313.patch \ | ||
19 | file://CVE-2022-25313-regression.patch \ | ||
18 | file://libtool-tag.patch \ | 20 | file://libtool-tag.patch \ |
19 | " | 21 | " |
20 | 22 | ||