xref: /third_party/libxml2/backport-malloc-fail-Fix-memory-leak-after-xmlRegNewState.patch

From e60c9f4c4b76da72772bfb6bb1f705e02fbb5324 Mon Sep 17 00:00:00 2001
From: Nick Wellnhofer <wellnhofer@aevum.de>
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