[njs] XML: fixed memory leak when exception happens in node.addChild().

Dmitry Volyntsev xeioex at nginx.com
Wed Mar 1 04:37:11 UTC 2023


details:   https://hg.nginx.org/njs/rev/688c4b1d9626
branches:  
changeset: 2059:688c4b1d9626
user:      Dmitry Volyntsev <xeioex at nginx.com>
date:      Tue Feb 28 20:34:31 2023 -0800
description:
XML: fixed memory leak when exception happens in node.addChild().

The issue was introduced in 3891f338e2c9.

Found by Coverity (CID 1521487).

diffstat:

 external/njs_xml_module.c |  25 +++++++++++--------------
 1 files changed, 11 insertions(+), 14 deletions(-)

diffs (68 lines):

diff -r 4911271d5453 -r 688c4b1d9626 external/njs_xml_module.c
--- a/external/njs_xml_module.c	Tue Feb 28 00:26:45 2023 -0800
+++ b/external/njs_xml_module.c	Tue Feb 28 20:34:31 2023 -0800
@@ -708,7 +708,7 @@ static njs_int_t
 njs_xml_node_ext_add_child(njs_vm_t *vm, njs_value_t *args,
     njs_uint_t nargs, njs_index_t unused)
 {
-    xmlNode    *current, *node, *copy;
+    xmlNode    *current, *node, *copy, *rnode;
     njs_int_t  ret;
 
     current = njs_vm_external(vm, njs_xml_node_proto_id, njs_argument(args, 0));
@@ -717,26 +717,27 @@ njs_xml_node_ext_add_child(njs_vm_t *vm,
         return NJS_ERROR;
     }
 
+    node = njs_xml_external_node(vm, njs_arg(args, nargs, 1));
+    if (njs_slow_path(node == NULL)) {
+        njs_vm_error(vm, "node is not a XMLNode object");
+        return NJS_ERROR;
+    }
+
     copy = xmlDocCopyNode(current, current->doc, 1);
     if (njs_slow_path(copy == NULL)) {
         njs_vm_error(vm, "xmlDocCopyNode() failed");
         return NJS_ERROR;
     }
 
-    node = njs_xml_external_node(vm, njs_arg(args, nargs, 1));
-    if (njs_slow_path(node == NULL)) {
-        njs_vm_error(vm, "node is not a XMLNode object");
-        goto error;
-    }
-
     node = xmlDocCopyNode(node, current->doc, 1);
     if (njs_slow_path(node == NULL)) {
         njs_vm_error(vm, "xmlDocCopyNode() failed");
         goto error;
     }
 
-    node = xmlAddChild(copy, node);
-    if (njs_slow_path(node == NULL)) {
+    rnode = xmlAddChild(copy, node);
+    if (njs_slow_path(rnode == NULL)) {
+        xmlFreeNode(node);
         njs_vm_error(vm, "xmlAddChild() failed");
         goto error;
     }
@@ -744,7 +745,7 @@ njs_xml_node_ext_add_child(njs_vm_t *vm,
     ret = xmlReconciliateNs(current->doc, copy);
     if (njs_slow_path(ret == -1)) {
         njs_vm_error(vm, "xmlReconciliateNs() failed");
-        return NJS_ERROR;
+        goto error;
     }
 
     njs_value_undefined_set(njs_vm_retval(vm));
@@ -753,10 +754,6 @@ njs_xml_node_ext_add_child(njs_vm_t *vm,
 
 error:
 
-    if (node != NULL) {
-        xmlFreeNode(node);
-    }
-
     xmlFreeNode(copy);
 
     return NJS_ERROR;


More information about the nginx-devel mailing list