[njs] Fixed memory leak when exception happens at xml.exclusiveC14n().

[Dmitry Volyntsev]

details:   https://hg.nginx.org/njs/rev/948b167202b2
branches:  
changeset: 2034:948b167202b2
user:      Dmitry Volyntsev <xei...@nginx.com>
date:      Mon Jan 30 18:19:16 2023 -0800
description:
Fixed memory leak when exception happens at xml.exclusiveC14n().

Found by Coverity (CID 1520596).

diffstat:

 external/njs_xml_module.c |  45 ++++++++++++++++-----------------------------
 src/test/njs_unit_test.c  |   4 ++++
 2 files changed, 20 insertions(+), 29 deletions(-)

diffs (136 lines):

diff -r 4f1e0dcd3c91 -r 948b167202b2 external/njs_xml_module.c
--- a/external/njs_xml_module.c Mon Jan 30 17:43:59 2023 -0800
+++ b/external/njs_xml_module.c Mon Jan 30 18:19:16 2023 -0800
@@ -69,8 +69,7 @@ static njs_int_t njs_xml_node_ext_text(n
     njs_value_t *value, njs_value_t *setval, njs_value_t *retval);
 
 static njs_xml_nset_t *njs_xml_nset_create(njs_vm_t *vm, xmlDoc *doc,
-    xmlNodeSet *nodes, njs_xml_nset_type_t type);
-static njs_xml_nset_t *njs_xml_nset_children(njs_vm_t *vm, xmlNode *parent);
+    xmlNode *current, njs_xml_nset_type_t type);
 static njs_xml_nset_t *njs_xml_nset_add(njs_xml_nset_t *nset,
     njs_xml_nset_t *add);
 static void njs_xml_nset_cleanup(void *data);
@@ -971,7 +970,6 @@ njs_xml_ext_canonicalization(njs_vm_t *v
     njs_str_t        data, string;
     njs_chb_t        chain;
     njs_bool_t       comments;
-    xmlNodeSet       *nodes;
     njs_value_t      *excluding, *prefixes;
     njs_xml_doc_t    *tree;
     njs_xml_nset_t   *nset, *children;
@@ -997,17 +995,11 @@ njs_xml_ext_canonicalization(njs_vm_t *v
     buf = xmlOutputBufferCreateIO(njs_xml_buf_write_cb, NULL, &chain, NULL);
     if (njs_slow_path(buf == NULL)) {
         njs_vm_error(vm, "xmlOutputBufferCreateIO() failed");
-        goto error;
+        return NJS_ERROR;
     }
 
     comments = njs_value_bool(njs_arg(args, nargs, 3));
 
-    nodes = xmlXPathNodeSetCreate(current);
-    if (njs_slow_path(nodes == NULL)) {
-        njs_vm_memory_error(vm);
-        goto error;
-    }
-
     excluding = njs_arg(args, nargs, 2);
 
     if (!njs_value_is_null_or_undefined(excluding)) {
@@ -1017,13 +1009,14 @@ njs_xml_ext_canonicalization(njs_vm_t *v
             goto error;
         }
 
-        nset = njs_xml_nset_create(vm, current->doc, nodes,
+        nset = njs_xml_nset_create(vm, current->doc, current,
                                    XML_NSET_TREE_NO_COMMENTS);
         if (njs_slow_path(nset == NULL)) {
             goto error;
         }
 
-        children = njs_xml_nset_children(vm, node);
+        children = njs_xml_nset_create(vm, node->doc, node,
+                                       XML_NSET_TREE_INVERT);
         if (njs_slow_path(children == NULL)) {
             goto error;
         }
@@ -1031,7 +1024,7 @@ njs_xml_ext_canonicalization(njs_vm_t *v
         nset = njs_xml_nset_add(nset, children);
 
     } else {
-        nset = njs_xml_nset_create(vm, current->doc, nodes,
+        nset = njs_xml_nset_create(vm, current->doc, current,
                                    comments ? XML_NSET_TREE
                                             : XML_NSET_TREE_NO_COMMENTS);
         if (njs_slow_path(nset == NULL)) {
@@ -1088,6 +1081,8 @@ njs_xml_ext_canonicalization(njs_vm_t *v
 
 error:
 
+    (void) xmlOutputBufferClose(buf);
+
     njs_chb_destroy(&chain);
 
     return NJS_ERROR;
@@ -1176,9 +1171,10 @@ njs_xml_attr_ext_prop_handler(njs_vm_t *
 
 
 static njs_xml_nset_t *
-njs_xml_nset_create(njs_vm_t *vm, xmlDoc *doc, xmlNodeSet *nodes,
+njs_xml_nset_create(njs_vm_t *vm, xmlDoc *doc, xmlNode *current,
     njs_xml_nset_type_t type)
 {
+    xmlNodeSet        *nodes;
     njs_xml_nset_t    *nset;
     njs_mp_cleanup_t  *cln;
 
@@ -1194,6 +1190,12 @@ njs_xml_nset_create(njs_vm_t *vm, xmlDoc
         return NULL;
     }
 
+    nodes = xmlXPathNodeSetCreate(current);
+    if (njs_slow_path(nodes == NULL)) {
+        njs_vm_memory_error(vm);
+        return NULL;
+    }
+
     cln->handler = njs_xml_nset_cleanup;
     cln->data = nset;
 
@@ -1207,21 +1209,6 @@ njs_xml_nset_create(njs_vm_t *vm, xmlDoc
 
 
 static njs_xml_nset_t *
-njs_xml_nset_children(njs_vm_t *vm, xmlNode *parent)
-{
-    xmlNodeSet  *nodes;
-
-    nodes = xmlXPathNodeSetCreate(parent);
-    if (njs_slow_path(nodes == NULL)) {
-        njs_vm_memory_error(vm);
-        return NULL;
-    }
-
-    return njs_xml_nset_create(vm, parent->doc, nodes, XML_NSET_TREE_INVERT);
-}
-
-
-static njs_xml_nset_t *
 njs_xml_nset_add(njs_xml_nset_t *nset, njs_xml_nset_t *add)
 {
     if (nset == NULL) {
diff -r 4f1e0dcd3c91 -r 948b167202b2 src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c  Mon Jan 30 17:43:59 2023 -0800
+++ b/src/test/njs_unit_test.c  Mon Jan 30 18:19:16 2023 -0800
@@ -21545,6 +21545,10 @@ static njs_unit_test_t  njs_xml_test[] =
       njs_str("{\"note\":{\"$name\":\"note\",\"$tags\":"
               "[{\"$name\":\"to\",\"$attrs\":{\"b\":\"bar\",\"a\":\"foo\"},"
               
"\"$text\":\"Tove\"},{\"$name\":\"from\",\"$text\":\"Jani\"}]}}") },
+
+    { njs_str("var xml = require('xml');"
+              "var doc = xml.parse(`<r></r>`); xml.exclusiveC14n(doc, 1)"),
+      njs_str("Error: \"excluding\" argument is not a XMLNode object") },
 };
 
 
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel