From e60c9f4c4b76da72772bfb6bb1f705e02fbb5324 Mon Sep 17 00:00:00 2001 From: Nick Wellnhofer Date: Wed, 15 Feb 2023 01:00:03 +0100 Subject: [PATCH] malloc-fail: Fix memory leak after xmlRegNewState Invoke xmlRegNewState from xmlRegStatePush to simplify error handling. Found with libFuzzer, see #344. Reference:https://github.com/GNOME/libxml2/commit/e60c9f4c4b76da72772bfb6bb1f705e02fbb5324 Conflict:NA --- xmlregexp.c | 144 ++++++++++++++++++++++++++-------------------------- 1 file changed, 71 insertions(+), 73 deletions(-) diff --git a/xmlregexp.c b/xmlregexp.c index 657912e..fb2eadc 100644 --- a/xmlregexp.c +++ b/xmlregexp.c @@ -1455,33 +1455,31 @@ xmlRegStateAddTrans(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr state, xmlRegStateAddTransTo(ctxt, target, state->no); } -static int -xmlRegStatePush(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr state) { - if (state == NULL) return(-1); - if (ctxt->maxStates == 0) { - ctxt->maxStates = 4; - ctxt->states = (xmlRegStatePtr *) xmlMalloc(ctxt->maxStates * - sizeof(xmlRegStatePtr)); - if (ctxt->states == NULL) { - xmlRegexpErrMemory(ctxt, "adding state"); - ctxt->maxStates = 0; - return(-1); - } - } else if (ctxt->nbStates >= ctxt->maxStates) { +static xmlRegStatePtr +xmlRegStatePush(xmlRegParserCtxtPtr ctxt) { + xmlRegStatePtr state; + + if (ctxt->nbStates >= ctxt->maxStates) { + size_t newSize = ctxt->maxStates ? ctxt->maxStates * 2 : 4; xmlRegStatePtr *tmp; - ctxt->maxStates *= 2; - tmp = (xmlRegStatePtr *) xmlRealloc(ctxt->states, ctxt->maxStates * - sizeof(xmlRegStatePtr)); + + tmp = xmlRealloc(ctxt->states, newSize * sizeof(tmp[0])); if (tmp == NULL) { xmlRegexpErrMemory(ctxt, "adding state"); - ctxt->maxStates /= 2; - return(-1); + return(NULL); } ctxt->states = tmp; + ctxt->maxStates = newSize; } + + state = xmlRegNewState(ctxt); + if (state == NULL) + return(NULL); + state->no = ctxt->nbStates; ctxt->states[ctxt->nbStates++] = state; - return(0); + + return(state); } /** @@ -1492,19 +1490,21 @@ xmlRegStatePush(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr state) { * @lax: * */ -static void +static int xmlFAGenerateAllTransition(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr from, xmlRegStatePtr to, int lax) { if (to == NULL) { - to = xmlRegNewState(ctxt); - xmlRegStatePush(ctxt, to); + to = xmlRegStatePush(ctxt); + if (to == NULL) + return(-1); ctxt->state = to; } if (lax) xmlRegStateAddTrans(ctxt, from, NULL, to, -1, REGEXP_ALL_LAX_COUNTER); else xmlRegStateAddTrans(ctxt, from, NULL, to, -1, REGEXP_ALL_COUNTER); + return(0); } /** @@ -1514,15 +1514,17 @@ xmlFAGenerateAllTransition(xmlRegParserCtxtPtr ctxt, * @to: the target state or NULL for building a new one * */ -static void +static int xmlFAGenerateEpsilonTransition(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr from, xmlRegStatePtr to) { if (to == NULL) { - to = xmlRegNewState(ctxt); - xmlRegStatePush(ctxt, to); + to = xmlRegStatePush(ctxt); + if (to == NULL) + return(-1); ctxt->state = to; } xmlRegStateAddTrans(ctxt, from, NULL, to, -1, -1); + return(0); } /** @@ -1533,15 +1535,17 @@ xmlFAGenerateEpsilonTransition(xmlRegParserCtxtPtr ctxt, * counter: the counter for that transition * */ -static void +static int xmlFAGenerateCountedEpsilonTransition(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr from, xmlRegStatePtr to, int counter) { if (to == NULL) { - to = xmlRegNewState(ctxt); - xmlRegStatePush(ctxt, to); + to = xmlRegStatePush(ctxt); + if (to == NULL) + return(-1); ctxt->state = to; } xmlRegStateAddTrans(ctxt, from, NULL, to, counter, -1); + return(0); } /** @@ -1552,15 +1556,17 @@ xmlFAGenerateCountedEpsilonTransition(xmlRegParserCtxtPtr ctxt, * counter: the counter for that transition * */ -static void +static int xmlFAGenerateCountedTransition(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr from, xmlRegStatePtr to, int counter) { if (to == NULL) { - to = xmlRegNewState(ctxt); - xmlRegStatePush(ctxt, to); + to = xmlRegStatePush(ctxt); + if (to == NULL) + return(-1); ctxt->state = to; } xmlRegStateAddTrans(ctxt, from, NULL, to, -1, counter); + return(0); } /** @@ -1599,8 +1605,9 @@ xmlFAGenerateTransitions(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr from, #ifdef DV } else if ((to == NULL) && (atom->quant != XML_REGEXP_QUANT_RANGE) && (atom->quant != XML_REGEXP_QUANT_ONCE)) { - to = xmlRegNewState(ctxt); - xmlRegStatePush(ctxt, to); + to = xmlRegStatePush(ctxt, to); + if (to == NULL) + return(-1); ctxt->state = to; xmlFAGenerateEpsilonTransition(ctxt, atom->stop, to); #endif @@ -1640,8 +1647,9 @@ xmlFAGenerateTransitions(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr from, if (to != NULL) { newstate = to; } else { - newstate = xmlRegNewState(ctxt); - xmlRegStatePush(ctxt, newstate); + newstate = xmlRegStatePush(ctxt); + if (newstate == NULL) + return(-1); } /* @@ -1721,12 +1729,9 @@ xmlFAGenerateTransitions(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr from, * we can discard the atom and generate an epsilon transition instead */ if (to == NULL) { - to = xmlRegNewState(ctxt); - if (to != NULL) - xmlRegStatePush(ctxt, to); - else { + to = xmlRegStatePush(ctxt); + if (to == NULL) return(-1); - } } xmlFAGenerateEpsilonTransition(ctxt, from, to); ctxt->state = to; @@ -1734,12 +1739,9 @@ xmlFAGenerateTransitions(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr from, return(0); } if (to == NULL) { - to = xmlRegNewState(ctxt); - if (to != NULL) - xmlRegStatePush(ctxt, to); - else { + to = xmlRegStatePush(ctxt); + if (to == NULL) return(-1); - } } end = to; if ((atom->quant == XML_REGEXP_QUANT_MULT) || @@ -1751,12 +1753,9 @@ xmlFAGenerateTransitions(xmlRegParserCtxtPtr ctxt, xmlRegStatePtr from, */ xmlRegStatePtr tmp; - tmp = xmlRegNewState(ctxt); - if (tmp != NULL) - xmlRegStatePush(ctxt, tmp); - else { + tmp = xmlRegStatePush(ctxt); + if (tmp == NULL) return(-1); - } xmlFAGenerateEpsilonTransition(ctxt, tmp, to); to = tmp; } @@ -5562,9 +5561,11 @@ xmlRegexpCompile(const xmlChar *regexp) { return(NULL); /* initialize the parser */ + ctxt->state = xmlRegStatePush(ctxt); + if (ctxt->state == NULL) + return(NULL); + ctxt->start = ctxt->state; ctxt->end = NULL; - ctxt->start = ctxt->state = xmlRegNewState(ctxt); - xmlRegStatePush(ctxt, ctxt->start); /* parse the expression building an automata */ xmlFAParseRegExp(ctxt, 1); @@ -5712,18 +5713,15 @@ xmlNewAutomata(void) { return(NULL); /* initialize the parser */ - ctxt->end = NULL; - ctxt->start = ctxt->state = xmlRegNewState(ctxt); - if (ctxt->start == NULL) { + ctxt->state = xmlRegStatePush(ctxt); + if (ctxt->state == NULL) { xmlFreeAutomata(ctxt); return(NULL); } + ctxt->start = ctxt->state; + ctxt->end = NULL; + ctxt->start->type = XML_REGEXP_START_STATE; - if (xmlRegStatePush(ctxt, ctxt->start) < 0) { - xmlRegFreeState(ctxt->start); - xmlFreeAutomata(ctxt); - return(NULL); - } ctxt->flags = 0; return(ctxt); @@ -6021,8 +6019,9 @@ xmlAutomataNewCountTrans2(xmlAutomataPtr am, xmlAutomataStatePtr from, /* xmlFAGenerateTransitions(am, from, to, atom); */ if (to == NULL) { - to = xmlRegNewState(am); - xmlRegStatePush(am, to); + to = xmlRegStatePush(am); + if (to == NULL) + return(NULL); } xmlRegStateAddTrans(am, from, atom, to, counter, -1); xmlRegAtomPush(am, atom); @@ -6087,8 +6086,9 @@ xmlAutomataNewCountTrans(xmlAutomataPtr am, xmlAutomataStatePtr from, /* xmlFAGenerateTransitions(am, from, to, atom); */ if (to == NULL) { - to = xmlRegNewState(am); - xmlRegStatePush(am, to); + to = xmlRegStatePush(am); + if (to == NULL) + return(NULL); } xmlRegStateAddTrans(am, from, atom, to, counter, -1); xmlRegAtomPush(am, atom); @@ -6173,8 +6173,9 @@ xmlAutomataNewOnceTrans2(xmlAutomataPtr am, xmlAutomataStatePtr from, /* xmlFAGenerateTransitions(am, from, to, atom); */ if (to == NULL) { - to = xmlRegNewState(am); - xmlRegStatePush(am, to); + to = xmlRegStatePush(am); + if (to == NULL) + return(NULL); } xmlRegStateAddTrans(am, from, atom, to, counter, -1); xmlRegAtomPush(am, atom); @@ -6232,8 +6233,9 @@ xmlAutomataNewOnceTrans(xmlAutomataPtr am, xmlAutomataStatePtr from, /* xmlFAGenerateTransitions(am, from, to, atom); */ if (to == NULL) { - to = xmlRegNewState(am); - xmlRegStatePush(am, to); + to = xmlRegStatePush(am); + if (to == NULL) + return(NULL); } xmlRegStateAddTrans(am, from, atom, to, counter, -1); xmlRegAtomPush(am, atom); @@ -6251,13 +6253,9 @@ xmlAutomataNewOnceTrans(xmlAutomataPtr am, xmlAutomataStatePtr from, */ xmlAutomataStatePtr xmlAutomataNewState(xmlAutomataPtr am) { - xmlAutomataStatePtr to; - if (am == NULL) return(NULL); - to = xmlRegNewState(am); - xmlRegStatePush(am, to); - return(to); + return(xmlRegStatePush(am)); } /** -- 2.27.0