1From e60c9f4c4b76da72772bfb6bb1f705e02fbb5324 Mon Sep 17 00:00:00 2001 2From: Nick Wellnhofer <wellnhofer@aevum.de> 3Date: Wed, 15 Feb 2023 01:00:03 +0100 4Subject: [PATCH] malloc-fail: Fix memory leak after xmlRegNewState 5 6Invoke xmlRegNewState from xmlRegStatePush to simplify error handling. 7 8Found with libFuzzer, see #344. 9 10Reference:https://github.com/GNOME/libxml2/commit/e60c9f4c4b76da72772bfb6bb1f705e02fbb5324 11Conflict:NA 12--- 13 xmlregexp.c | 144 ++++++++++++++++++++++++++-------------------------- 14 1 file changed, 71 insertions(+), 73 deletions(-) 15 16diff --git a/xmlregexp.c b/xmlregexp.c 17index 657912e..fb2eadc 100644 18--- a/xmlregexp.c 19+++ b/xmlregexp.c 20@@ -1455,33 +1455,31 @@ xmlRegStateAddTrans(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr state, 21 xmlRegStateAddTransTo(ctxt, target, state->no); 22 } 23 24-static int 25-xmlRegStatePush(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr state) { 26- if (state == NULL) return(-1); 27- if (ctxt->maxStates == 0) { 28- ctxt->maxStates = 4; 29- ctxt->states = (xmlRegStatePtr *) xmlMalloc(ctxt->maxStates * 30- sizeof(xmlRegStatePtr)); 31- if (ctxt->states == NULL) { 32- xmlRegexpErrMemory(ctxt, "adding state"); 33- ctxt->maxStates = 0; 34- return(-1); 35- } 36- } else if (ctxt->nbStates >= ctxt->maxStates) { 37+static xmlRegStatePtr 38+xmlRegStatePush(xmlRegParserCtxtPtr ctxt) { 39+ xmlRegStatePtr state; 40+ 41+ if (ctxt->nbStates >= ctxt->maxStates) { 42+ size_t newSize = ctxt->maxStates ? ctxt->maxStates * 2 : 4; 43 xmlRegStatePtr *tmp; 44- ctxt->maxStates *= 2; 45- tmp = (xmlRegStatePtr *) xmlRealloc(ctxt->states, ctxt->maxStates * 46- sizeof(xmlRegStatePtr)); 47+ 48+ tmp = xmlRealloc(ctxt->states, newSize * sizeof(tmp[0])); 49 if (tmp == NULL) { 50 xmlRegexpErrMemory(ctxt, "adding state"); 51- ctxt->maxStates /= 2; 52- return(-1); 53+ return(NULL); 54 } 55 ctxt->states = tmp; 56+ ctxt->maxStates = newSize; 57 } 58+ 59+ state = xmlRegNewState(ctxt); 60+ if (state == NULL) 61+ return(NULL); 62+ 63 state->no = ctxt->nbStates; 64 ctxt->states[ctxt->nbStates++] = state; 65- return(0); 66+ 67+ return(state); 68 } 69 70 /** 71@@ -1492,19 +1490,21 @@ xmlRegStatePush(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr state) { 72 * @lax: 73 * 74 */ 75-static void 76+static int 77 xmlFAGenerateAllTransition(xmlRegParserCtxtPtr ctxt, 78 xmlRegStatePtr from, xmlRegStatePtr to, 79 int lax) { 80 if (to == NULL) { 81- to = xmlRegNewState(ctxt); 82- xmlRegStatePush(ctxt, to); 83+ to = xmlRegStatePush(ctxt); 84+ if (to == NULL) 85+ return(-1); 86 ctxt->state = to; 87 } 88 if (lax) 89 xmlRegStateAddTrans(ctxt, from, NULL, to, -1, REGEXP_ALL_LAX_COUNTER); 90 else 91 xmlRegStateAddTrans(ctxt, from, NULL, to, -1, REGEXP_ALL_COUNTER); 92+ return(0); 93 } 94 95 /** 96@@ -1514,15 +1514,17 @@ xmlFAGenerateAllTransition(xmlRegParserCtxtPtr ctxt, 97 * @to: the target state or NULL for building a new one 98 * 99 */ 100-static void 101+static int 102 xmlFAGenerateEpsilonTransition(xmlRegParserCtxtPtr ctxt, 103 xmlRegStatePtr from, xmlRegStatePtr to) { 104 if (to == NULL) { 105- to = xmlRegNewState(ctxt); 106- xmlRegStatePush(ctxt, to); 107+ to = xmlRegStatePush(ctxt); 108+ if (to == NULL) 109+ return(-1); 110 ctxt->state = to; 111 } 112 xmlRegStateAddTrans(ctxt, from, NULL, to, -1, -1); 113+ return(0); 114 } 115 116 /** 117@@ -1533,15 +1535,17 @@ xmlFAGenerateEpsilonTransition(xmlRegParserCtxtPtr ctxt, 118 * counter: the counter for that transition 119 * 120 */ 121-static void 122+static int 123 xmlFAGenerateCountedEpsilonTransition(xmlRegParserCtxtPtr ctxt, 124 xmlRegStatePtr from, xmlRegStatePtr to, int counter) { 125 if (to == NULL) { 126- to = xmlRegNewState(ctxt); 127- xmlRegStatePush(ctxt, to); 128+ to = xmlRegStatePush(ctxt); 129+ if (to == NULL) 130+ return(-1); 131 ctxt->state = to; 132 } 133 xmlRegStateAddTrans(ctxt, from, NULL, to, counter, -1); 134+ return(0); 135 } 136 137 /** 138@@ -1552,15 +1556,17 @@ xmlFAGenerateCountedEpsilonTransition(xmlRegParserCtxtPtr ctxt, 139 * counter: the counter for that transition 140 * 141 */ 142-static void 143+static int 144 xmlFAGenerateCountedTransition(xmlRegParserCtxtPtr ctxt, 145 xmlRegStatePtr from, xmlRegStatePtr to, int counter) { 146 if (to == NULL) { 147- to = xmlRegNewState(ctxt); 148- xmlRegStatePush(ctxt, to); 149+ to = xmlRegStatePush(ctxt); 150+ if (to == NULL) 151+ return(-1); 152 ctxt->state = to; 153 } 154 xmlRegStateAddTrans(ctxt, from, NULL, to, -1, counter); 155+ return(0); 156 } 157 158 /** 159@@ -1599,8 +1605,9 @@ xmlFAGenerateTransitions(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr from, 160 #ifdef DV 161 } else if ((to == NULL) && (atom->quant != XML_REGEXP_QUANT_RANGE) && 162 (atom->quant != XML_REGEXP_QUANT_ONCE)) { 163- to = xmlRegNewState(ctxt); 164- xmlRegStatePush(ctxt, to); 165+ to = xmlRegStatePush(ctxt, to); 166+ if (to == NULL) 167+ return(-1); 168 ctxt->state = to; 169 xmlFAGenerateEpsilonTransition(ctxt, atom->stop, to); 170 #endif 171@@ -1640,8 +1647,9 @@ xmlFAGenerateTransitions(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr from, 172 if (to != NULL) { 173 newstate = to; 174 } else { 175- newstate = xmlRegNewState(ctxt); 176- xmlRegStatePush(ctxt, newstate); 177+ newstate = xmlRegStatePush(ctxt); 178+ if (newstate == NULL) 179+ return(-1); 180 } 181 182 /* 183@@ -1721,12 +1729,9 @@ xmlFAGenerateTransitions(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr from, 184 * we can discard the atom and generate an epsilon transition instead 185 */ 186 if (to == NULL) { 187- to = xmlRegNewState(ctxt); 188- if (to != NULL) 189- xmlRegStatePush(ctxt, to); 190- else { 191+ to = xmlRegStatePush(ctxt); 192+ if (to == NULL) 193 return(-1); 194- } 195 } 196 xmlFAGenerateEpsilonTransition(ctxt, from, to); 197 ctxt->state = to; 198@@ -1734,12 +1739,9 @@ xmlFAGenerateTransitions(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr from, 199 return(0); 200 } 201 if (to == NULL) { 202- to = xmlRegNewState(ctxt); 203- if (to != NULL) 204- xmlRegStatePush(ctxt, to); 205- else { 206+ to = xmlRegStatePush(ctxt); 207+ if (to == NULL) 208 return(-1); 209- } 210 } 211 end = to; 212 if ((atom->quant == XML_REGEXP_QUANT_MULT) || 213@@ -1751,12 +1753,9 @@ xmlFAGenerateTransitions(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr from, 214 */ 215 xmlRegStatePtr tmp; 216 217- tmp = xmlRegNewState(ctxt); 218- if (tmp != NULL) 219- xmlRegStatePush(ctxt, tmp); 220- else { 221+ tmp = xmlRegStatePush(ctxt); 222+ if (tmp == NULL) 223 return(-1); 224- } 225 xmlFAGenerateEpsilonTransition(ctxt, tmp, to); 226 to = tmp; 227 } 228@@ -5562,9 +5561,11 @@ xmlRegexpCompile(const xmlChar *regexp) { 229 return(NULL); 230 231 /* initialize the parser */ 232+ ctxt->state = xmlRegStatePush(ctxt); 233+ if (ctxt->state == NULL) 234+ return(NULL); 235+ ctxt->start = ctxt->state; 236 ctxt->end = NULL; 237- ctxt->start = ctxt->state = xmlRegNewState(ctxt); 238- xmlRegStatePush(ctxt, ctxt->start); 239 240 /* parse the expression building an automata */ 241 xmlFAParseRegExp(ctxt, 1); 242@@ -5712,18 +5713,15 @@ xmlNewAutomata(void) { 243 return(NULL); 244 245 /* initialize the parser */ 246- ctxt->end = NULL; 247- ctxt->start = ctxt->state = xmlRegNewState(ctxt); 248- if (ctxt->start == NULL) { 249+ ctxt->state = xmlRegStatePush(ctxt); 250+ if (ctxt->state == NULL) { 251 xmlFreeAutomata(ctxt); 252 return(NULL); 253 } 254+ ctxt->start = ctxt->state; 255+ ctxt->end = NULL; 256+ 257 ctxt->start->type = XML_REGEXP_START_STATE; 258- if (xmlRegStatePush(ctxt, ctxt->start) < 0) { 259- xmlRegFreeState(ctxt->start); 260- xmlFreeAutomata(ctxt); 261- return(NULL); 262- } 263 ctxt->flags = 0; 264 265 return(ctxt); 266@@ -6021,8 +6019,9 @@ xmlAutomataNewCountTrans2(xmlAutomataPtr am, xmlAutomataStatePtr from, 267 268 /* xmlFAGenerateTransitions(am, from, to, atom); */ 269 if (to == NULL) { 270- to = xmlRegNewState(am); 271- xmlRegStatePush(am, to); 272+ to = xmlRegStatePush(am); 273+ if (to == NULL) 274+ return(NULL); 275 } 276 xmlRegStateAddTrans(am, from, atom, to, counter, -1); 277 xmlRegAtomPush(am, atom); 278@@ -6087,8 +6086,9 @@ xmlAutomataNewCountTrans(xmlAutomataPtr am, xmlAutomataStatePtr from, 279 280 /* xmlFAGenerateTransitions(am, from, to, atom); */ 281 if (to == NULL) { 282- to = xmlRegNewState(am); 283- xmlRegStatePush(am, to); 284+ to = xmlRegStatePush(am); 285+ if (to == NULL) 286+ return(NULL); 287 } 288 xmlRegStateAddTrans(am, from, atom, to, counter, -1); 289 xmlRegAtomPush(am, atom); 290@@ -6173,8 +6173,9 @@ xmlAutomataNewOnceTrans2(xmlAutomataPtr am, xmlAutomataStatePtr from, 291 292 /* xmlFAGenerateTransitions(am, from, to, atom); */ 293 if (to == NULL) { 294- to = xmlRegNewState(am); 295- xmlRegStatePush(am, to); 296+ to = xmlRegStatePush(am); 297+ if (to == NULL) 298+ return(NULL); 299 } 300 xmlRegStateAddTrans(am, from, atom, to, counter, -1); 301 xmlRegAtomPush(am, atom); 302@@ -6232,8 +6233,9 @@ xmlAutomataNewOnceTrans(xmlAutomataPtr am, xmlAutomataStatePtr from, 303 304 /* xmlFAGenerateTransitions(am, from, to, atom); */ 305 if (to == NULL) { 306- to = xmlRegNewState(am); 307- xmlRegStatePush(am, to); 308+ to = xmlRegStatePush(am); 309+ if (to == NULL) 310+ return(NULL); 311 } 312 xmlRegStateAddTrans(am, from, atom, to, counter, -1); 313 xmlRegAtomPush(am, atom); 314@@ -6251,13 +6253,9 @@ xmlAutomataNewOnceTrans(xmlAutomataPtr am, xmlAutomataStatePtr from, 315 */ 316 xmlAutomataStatePtr 317 xmlAutomataNewState(xmlAutomataPtr am) { 318- xmlAutomataStatePtr to; 319- 320 if (am == NULL) 321 return(NULL); 322- to = xmlRegNewState(am); 323- xmlRegStatePush(am, to); 324- return(to); 325+ return(xmlRegStatePush(am)); 326 } 327 328 /** 329-- 3302.27.0 331 332