details: https://hg.nginx.org/njs/rev/948b167202b2
branches:
changeset: 2034:948b167202b2
user: Dmitry Volyntsev <xeioex@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