Fix use-after-free bug in mxmlSet APIs (Issue #241)

CHANGES.md

@@ -8,6 +8,9 @@
 - Moved `mxmldoc` to a new `codedoc` project whose focus is on generating
   code documentation (Issue #235, Issue #236, Issue #237)
 - Error messages now include the line number of the error (Issue #230)
+- The `mxmlSetCDATA`, `mxmlSetElement`, `mxmlSetOpaque`, `mxmlSetOpaquef`,
+  `mxmlSetText`, and `mxmlSetTextf` functions caused a use-after-free bug if
+  the value came from the same node (Issue #241)
 
 
 # Changes in Mini-XML 2.12

mxml-set.c

@@ -29,6 +29,9 @@ int					/* O - 0 on success, -1 on failure */
 mxmlSetCDATA(mxml_node_t *node,		/* I - Node to set */
              const char  *data)		/* I - New data string */
 {
+  char	*s;				/* String pointer */
+
+
  /*
   * Range check input...
   */
@@ -43,14 +46,19 @@ mxmlSetCDATA(mxml_node_t *node,		/* I - Node to set */
       strncmp(node->value.element.name, "![CDATA[", 8))
     return (-1);
 
+  if (data == (node->value.element.name + 8))
+    return (0);
+
  /*
-  * Free any old element value and set the new value...
+  * Allocate the new value, free any old element value, and set the new value...
   */
 
+  s = _mxml_strdupf("![CDATA[%s", data);
+
   if (node->value.element.name)
     free(node->value.element.name);
 
-  node->value.element.name = _mxml_strdupf("![CDATA[%s", data);
+  node->value.element.name = s;
 
   return (0);
 }
@@ -81,6 +89,12 @@ mxmlSetCustom(
   if (!node || node->type != MXML_CUSTOM)
     return (-1);
 
+  if (data == node->value.custom.data)
+  {
+    node->value.custom.destroy = destroy;
+    return (0);
+  }
+
  /*
   * Free any old element value and set the new value...
   */
@@ -112,6 +126,9 @@ mxmlSetElement(mxml_node_t *node,	/* I - Node to set */
   if (!node || node->type != MXML_ELEMENT || !name)
     return (-1);
 
+  if (name == node->value.element.name)
+    return (0);
+
  /*
   * Free any old element value and set the new value...
   */
@@ -177,6 +194,9 @@ mxmlSetOpaque(mxml_node_t *node,	/* I - Node to set */
   if (!node || node->type != MXML_OPAQUE || !opaque)
     return (-1);
 
+  if (node->value.opaque == opaque)
+    return (0);
+
  /*
   * Free any old opaque value and set the new value...
   */
@@ -204,6 +224,7 @@ mxmlSetOpaquef(mxml_node_t *node,	/* I - Node to set */
 	       ...)			/* I - Additional arguments as needed */
 {
   va_list	ap;			/* Pointer to arguments */
+  char		*s;			/* Temporary string */
 
 
  /*
@@ -218,17 +239,17 @@ mxmlSetOpaquef(mxml_node_t *node,	/* I - Node to set */
     return (-1);
 
  /*
-  * Free any old string value and set the new value...
+  * Format the new string, free any old string value, and set the new value...
   */
 
-  if (node->value.opaque)
-    free(node->value.opaque);
-
   va_start(ap, format);
+  s = _mxml_strdupf(format, ap);
+  va_end(ap);
 
-  node->value.opaque = _mxml_strdupf(format, ap);
+  if (node->value.opaque)
+    free(node->value.opaque);
 
-  va_end(ap);
+  node->value.opaque = s;
 
   return (0);
 }
@@ -287,6 +308,12 @@ mxmlSetText(mxml_node_t *node,		/* I - Node to set */
   if (!node || node->type != MXML_TEXT || !string)
     return (-1);
 
+  if (string == node->value.text.string)
+  {
+    node->value.text.whitespace = whitespace;
+    return (0);
+  }
+
  /*
   * Free any old string value and set the new value...
   */
@@ -314,6 +341,7 @@ mxmlSetTextf(mxml_node_t *node,		/* I - Node to set */
 	     ...)			/* I - Additional arguments as needed */
 {
   va_list	ap;			/* Pointer to arguments */
+  char		*s;			/* Temporary string */
 
 
  /*
@@ -331,15 +359,15 @@ mxmlSetTextf(mxml_node_t *node,		/* I - Node to set */
   * Free any old string value and set the new value...
   */
 
+  va_start(ap, format);
+  s = _mxml_strdupf(format, ap);
+  va_end(ap);
+
   if (node->value.text.string)
     free(node->value.text.string);
 
-  va_start(ap, format);
-
   node->value.text.whitespace = whitespace;
-  node->value.text.string     = _mxml_strdupf(format, ap);
-
-  va_end(ap);
+  node->value.text.string     = s;
 
   return (0);
 }