From: Dmitry Volyntsev Date: Thu, 13 Aug 2026 03:58:26 +0000 (-0700) Subject: XML: fix off-by-one in namespace prefix list parsing X-Git-Tag: 1.0.1~2 X-Git-Url: https://git.kaiwu.me/%7B@url%7D?a=commitdiff_plain;h=1308b320821aa209a7985cc8f52c09a515130ba6;p=njs.git XML: fix off-by-one in namespace prefix list parsing njs_xml_parse_ns_list() allocated exactly size pointers, but the terminating NULL was stored at the index equal to the number of parsed tokens. When the token count matched the current capacity, the terminator was written one pointer past the allocation. The growing loop is replaced with a single allocation holding the array of pointers followed by the writable copy of the list, the number of tokens is counted beforehand. The list is split on any XML whitespace character, empty tokens are not produced. Also, fixed potential in place modification of QuickJS string value. Reported by Vladimir Vulnerability Research Tech Lead @ Cyera, evilgensec --- diff --git a/external/njs_xml_module.c b/external/njs_xml_module.c index 4736c3ac..f6687ad8 100644 --- a/external/njs_xml_module.c +++ b/external/njs_xml_module.c @@ -1663,60 +1663,70 @@ njs_xml_c14n_visibility_cb(void *user_data, xmlNode *node, xmlNode *parent) } +/* the C14N prefix list is a whitespace delimited list */ +#define njs_xml_ns_sep(c) \ + ((c) == ' ' || (c) == '\t' || (c) == '\n' || (c) == '\r') + + static u_char ** njs_xml_parse_ns_list(njs_vm_t *vm, njs_str_t *src) { - u_char *p, **buf, **n, **out; - size_t size, idx; + size_t n; + u_char *p, *end, *dst, **buf, **out; + + /* + * The pointers and the writable copy of the list are allocated as a single + * block, the copy follows the NULL terminated array of pointers to it. + * The tokens are counted exactly the way the list is split below. + */ + + n = 0; + end = src->start + src->length; - out = NULL; + for (p = src->start; p < end && *p != '\0'; /* void */) { + if (njs_xml_ns_sep(*p)) { + p++; + continue; + } + + n++; + + while (p < end && *p != '\0' && !njs_xml_ns_sep(*p)) { + p++; + } + } - p = njs_mp_alloc(njs_vm_memory_pool(vm), src->length + 1); - if (njs_slow_path(p == NULL)) { + if (njs_slow_path(n >= (SIZE_MAX - src->length - 1) / sizeof(u_char *))) { njs_vm_memory_error(vm); return NULL; } - memcpy(p, src->start, src->length); - p[src->length] = '\0'; - - size = 8; - - buf = njs_mp_alloc(njs_vm_memory_pool(vm), size * sizeof(char *)); + buf = njs_mp_alloc(njs_vm_memory_pool(vm), + (n + 1) * sizeof(u_char *) + src->length + 1); if (njs_slow_path(buf == NULL)) { njs_vm_memory_error(vm); return NULL; } - out = buf; + dst = (u_char *) &buf[n + 1]; - while (*p != '\0') { - idx = out - buf; - - if (idx >= size) { - size *= 2; - - n = njs_mp_alloc(njs_vm_memory_pool(vm), size * sizeof(char *)); - if (njs_slow_path(buf == NULL)) { - njs_vm_memory_error(vm); - return NULL; - } + memcpy(dst, src->start, src->length); + dst[src->length] = '\0'; - memcpy(n, buf, size * sizeof(char *) / 2); - buf = n; + out = buf; + p = dst; - out = &buf[idx]; + while (*p != '\0') { + if (njs_xml_ns_sep(*p)) { + *p++ = '\0'; + continue; } *out++ = p; - while (*p != ' ' && *p != '\0') { + while (*p != '\0' && !njs_xml_ns_sep(*p)) { p++; } - - if (*p == ' ') { - *p++ = '\0'; - } } *out = NULL; diff --git a/external/qjs_xml_module.c b/external/qjs_xml_module.c index ebb87657..408b6b04 100644 --- a/external/qjs_xml_module.c +++ b/external/qjs_xml_module.c @@ -100,7 +100,7 @@ static int qjs_xml_attr_get_own_property_names(JSContext *cx, JSPropertyEnum **ptab, uint32_t *plen, JSValueConst obj); static void qjs_xml_attr_finalizer(JSRuntime *rt, JSValue val); -static u_char **qjs_xml_parse_ns_list(JSContext *cx, u_char *src); +static u_char **qjs_xml_parse_ns_list(JSContext *cx, njs_str_t *src); static int qjs_xml_c14n_visibility_cb(void *user_data, xmlNode *node, xmlNode *parent); static int qjs_xml_buf_write_cb(void *context, const char *buffer, int len); @@ -245,11 +245,12 @@ qjs_xml_canonicalization(JSContext *cx, JSValueConst this_val, int argc, JSValueConst *argv, int magic) { int comments; - u_char **prefix_list, *pref; + u_char **prefix_list; xmlDoc *doc; xmlNode *node; ssize_t size; JSValue excluding, prefixes, ret; + njs_str_t pref; njs_chb_t chain; qjs_xml_node_t *nd; qjs_xml_nset_t *nset, *children; @@ -309,13 +310,15 @@ qjs_xml_canonicalization(JSContext *cx, JSValueConst this_val, int argc, goto fail; } - pref = (u_char *) JS_ToCString(cx, prefixes); - if (pref == NULL) { - JS_ThrowOutOfMemory(cx); + pref.start = (u_char *) JS_ToCStringLen(cx, &pref.length, prefixes); + if (pref.start == NULL) { goto fail; } - prefix_list = qjs_xml_parse_ns_list(cx, pref); + prefix_list = qjs_xml_parse_ns_list(cx, &pref); + + JS_FreeCString(cx, (char *) pref.start); + if (prefix_list == NULL) { goto fail; } @@ -1720,47 +1723,68 @@ qjs_xml_attr_finalizer(JSRuntime *rt, JSValue val) } +/* the C14N prefix list is a whitespace delimited list */ +#define qjs_xml_ns_sep(c) \ + ((c) == ' ' || (c) == '\t' || (c) == '\n' || (c) == '\r') + + static u_char ** -qjs_xml_parse_ns_list(JSContext *cx, u_char *src) +qjs_xml_parse_ns_list(JSContext *cx, njs_str_t *src) { - u_char *p, **buf, **out; - size_t size, idx; + size_t n; + u_char *p, *end, *dst, **buf, **out; - size = 8; - p = src; + /* + * The pointers and the writable copy of the list are allocated as a single + * block, the copy follows the NULL terminated array of pointers to it. + * The tokens are counted exactly the way the list is split below. + */ - buf = js_mallocz(cx, size * sizeof(char *)); - if (buf == NULL) { + n = 0; + end = src->start + src->length; + + for (p = src->start; p < end && *p != '\0'; /* void */) { + if (qjs_xml_ns_sep(*p)) { + p++; + continue; + } + + n++; + + while (p < end && *p != '\0' && !qjs_xml_ns_sep(*p)) { + p++; + } + } + + if (n >= (SIZE_MAX - src->length - 1) / sizeof(u_char *)) { JS_ThrowOutOfMemory(cx); return NULL; } - out = buf; + buf = js_malloc(cx, (n + 1) * sizeof(u_char *) + src->length + 1); + if (buf == NULL) { + return NULL; + } - while (*p != '\0') { - idx = out - buf; + dst = (u_char *) &buf[n + 1]; - if (idx >= size) { - size *= 2; + memcpy(dst, src->start, src->length); + dst[src->length] = '\0'; - buf = js_realloc(cx, buf, size * sizeof(char *)); - if (buf == NULL) { - JS_ThrowOutOfMemory(cx); - return NULL; - } + out = buf; + p = dst; - out = &buf[idx]; + while (*p != '\0') { + if (qjs_xml_ns_sep(*p)) { + *p++ = '\0'; + continue; } *out++ = p; - while (*p != ' ' && *p != '\0') { + while (*p != '\0' && !qjs_xml_ns_sep(*p)) { p++; } - - if (*p == ' ') { - *p++ = '\0'; - } } *out = NULL; diff --git a/test/xml/xml.t.mjs b/test/xml/xml.t.mjs index 8590bddc..75ce9c4c 100644 --- a/test/xml/xml.t.mjs +++ b/test/xml/xml.t.mjs @@ -127,6 +127,34 @@ let c14n_tsuite = { expected: `foo` }, { call: (doc) => xml.exclusiveC14n(doc.pdu.elem1, null, false, 'a b c d e f g h i j'), expected: `foo` }, + { call: (doc) => xml.exclusiveC14n(doc.pdu.elem1, null, false, + 'a '.repeat(7) + 'n0'), + expected: `foo` }, + { call: (doc) => xml.exclusiveC14n(doc.pdu.elem1, null, false, + 'a '.repeat(15) + 'n0'), + expected: `foo` }, + { call: (doc) => xml.exclusiveC14n(doc.pdu.elem1, null, false, + 'a '.repeat(127) + 'n0'), + expected: `foo` }, + { call: (doc) => xml.exclusiveC14n(doc.pdu.elem1, null, false, + ' n0 a '), + expected: `foo` }, + { call: (doc) => xml.exclusiveC14n(doc.pdu.elem1, null, false, + 'a\tn0\r\nb'), + expected: `foo` }, + { doc: `x`, + call: (doc) => xml.exclusiveC14n(doc.r.b, null, false, '#default'), + expected: `x` }, + { doc: `x`, + call: (doc) => xml.exclusiveC14n(doc.r.b, null, false, ' a b '), + expected: `x` }, + { call: (doc) => { + let prefixes = 'n0 n1'; + xml.exclusiveC14n(doc.pdu.elem1, null, false, prefixes); + return prefixes; + }, + buffer: false, + expected: 'n0 n1' }, { doc: `ToveJani`, call: (doc) => xml.c14n(doc.note), expected: `ToveJani` },