[njs] Modules: fixed name corruption in variable and header processing.
noreply at nginx.com
noreply at nginx.com
Tue Feb 11 01:51:01 UTC 2025
details: https://github.com/nginx/njs/commit/ae7d4f42d5d7497e6e8d3d30ff5aebfba228d27c
branches: master
commit: ae7d4f42d5d7497e6e8d3d30ff5aebfba228d27c
user: Dmitry Volyntsev <xeioex at nginx.com>
date: Fri, 7 Feb 2025 17:23:09 -0800
description:
Modules: fixed name corruption in variable and header processing.
The HTTP and Stream JS modules were performing in-place lowercasing of
variable and header names, which could inadvertently overwrite the
original data.
In the NJS engine, the problem did not manifest itself for strings up to
14 bytes long because they are inlined into the value.
---
nginx/ngx_http_js_module.c | 106 ++++++++++++++++++++++++++++++++++---------
nginx/ngx_stream_js_module.c | 72 ++++++++++++++++++++++++-----
nginx/t/js_headers.t | 16 ++++++-
nginx/t/js_variables.t | 26 ++++++++++-
nginx/t/stream_js_var.t | 33 +++++++++++++-
5 files changed, 213 insertions(+), 40 deletions(-)
diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
index 66cb97c0..7cd87401 100644
--- a/nginx/ngx_http_js_module.c
+++ b/nginx/ngx_http_js_module.c
@@ -3222,6 +3222,7 @@ ngx_http_js_request_variables(njs_vm_t *vm, njs_object_prop_t *prop,
ngx_http_variable_t *v;
ngx_http_core_main_conf_t *cmcf;
ngx_http_variable_value_t *vv;
+ u_char storage[64];
rc = njs_vm_prop_name(vm, prop, &val);
if (rc != NJS_OK) {
@@ -3229,20 +3230,17 @@ ngx_http_js_request_variables(njs_vm_t *vm, njs_object_prop_t *prop,
return NJS_DECLINED;
}
- name.data = val.start;
- name.len = val.length;
-
if (setval == NULL) {
is_capture = 1;
- for (i = 0; i < name.len; i++) {
- if (name.data[i] < '0' || name.data[i] > '9') {
+ for (i = 0; i < val.length; i++) {
+ if (val.start[i] < '0' || val.start[i] > '9') {
is_capture = 0;
break;
}
}
if (is_capture) {
- key = ngx_atoi(name.data, name.len) * 2;
+ key = ngx_atoi(val.start, val.length) * 2;
if (r->captures == NULL || r->captures_data == NULL
|| r->ncaptures <= key)
{
@@ -3258,7 +3256,21 @@ ngx_http_js_request_variables(njs_vm_t *vm, njs_object_prop_t *prop,
}
/* Lookup the variable in nginx variables */
- key = ngx_hash_strlow(name.data, name.data, name.len);
+
+ if (val.length < sizeof(storage)) {
+ name.data = storage;
+
+ } else {
+ name.data = ngx_pnalloc(r->pool, val.length);
+ if (name.data == NULL) {
+ njs_vm_memory_error(vm);
+ return NJS_ERROR;
+ }
+ }
+
+ name.len = val.length;
+
+ key = ngx_hash_strlow(name.data, val.start, val.length);
vv = ngx_http_get_variable(r, &name, key);
if (vv == NULL || vv->not_found) {
@@ -3272,9 +3284,20 @@ ngx_http_js_request_variables(njs_vm_t *vm, njs_object_prop_t *prop,
cmcf = ngx_http_get_module_main_conf(r, ngx_http_core_module);
- key = ngx_hash_strlow(name.data, name.data, name.len);
+ if (val.length < sizeof(storage)) {
+ name.data = storage;
+
+ } else {
+ name.data = ngx_pnalloc(r->pool, val.length);
+ if (name.data == NULL) {
+ njs_vm_memory_error(vm);
+ return NJS_ERROR;
+ }
+ }
+
+ key = ngx_hash_strlow(name.data, val.start, val.length);
- v = ngx_hash_find(&cmcf->variables_hash, key, name.data, name.len);
+ v = ngx_hash_find(&cmcf->variables_hash, key, name.data, val.length);
if (v == NULL) {
njs_vm_error(vm, "variable not found");
@@ -3783,6 +3806,7 @@ ngx_http_js_header_in(njs_vm_t *vm, ngx_http_request_t *r, unsigned flags,
ngx_table_elt_t **ph;
ngx_http_header_t *hh;
ngx_http_core_main_conf_t *cmcf;
+ u_char storage[128];
if (retval == NULL) {
return NJS_OK;
@@ -3790,10 +3814,15 @@ ngx_http_js_header_in(njs_vm_t *vm, ngx_http_request_t *r, unsigned flags,
/* look up hashed headers */
- lowcase_key = ngx_pnalloc(r->pool, name->length);
- if (lowcase_key == NULL) {
- njs_vm_memory_error(vm);
- return NJS_ERROR;
+ if (name->length < sizeof(storage)) {
+ lowcase_key = storage;
+
+ } else {
+ lowcase_key = ngx_pnalloc(r->pool, name->length);
+ if (lowcase_key == NULL) {
+ njs_vm_memory_error(vm);
+ return NJS_ERROR;
+ }
}
hash = ngx_hash_strlow(lowcase_key, name->start, name->length);
@@ -6130,10 +6159,11 @@ ngx_http_qjs_variables_own_property(JSContext *cx, JSPropertyDescriptor *pdesc,
JSValueConst obj, JSAtom prop)
{
uint32_t buffer_type;
- ngx_str_t name;
+ ngx_str_t name, name_lc;
ngx_uint_t i, key, start, length, is_capture;
ngx_http_request_t *r;
ngx_http_variable_value_t *vv;
+ u_char storage[64];
r = JS_GetOpaque(obj, NGX_QJS_CLASS_ID_HTTP_VARS);
@@ -6184,9 +6214,22 @@ ngx_http_qjs_variables_own_property(JSContext *cx, JSPropertyDescriptor *pdesc,
return 1;
}
- key = ngx_hash_strlow(name.data, name.data, name.len);
+ if (name.len < sizeof(storage)) {
+ name_lc.data = storage;
+
+ } else {
+ name_lc.data = ngx_pnalloc(r->pool, name.len);
+ if (name_lc.data == NULL) {
+ (void) JS_ThrowOutOfMemory(cx);
+ return -1;
+ }
+ }
+
+ name_lc.len = name.len;
+
+ key = ngx_hash_strlow(name_lc.data, name.data, name.len);
- vv = ngx_http_get_variable(r, &name, key);
+ vv = ngx_http_get_variable(r, &name_lc, key);
JS_FreeCString(cx, (char *) name.data);
if (vv == NULL || vv->not_found) {
return 0;
@@ -6207,6 +6250,7 @@ static int
ngx_http_qjs_variables_set_property(JSContext *cx, JSValueConst obj,
JSAtom prop, JSValueConst value, JSValueConst receiver, int flags)
{
+ u_char *lowcase_key;
ngx_str_t name, s;
ngx_uint_t key;
ngx_http_js_ctx_t *ctx;
@@ -6214,6 +6258,7 @@ ngx_http_qjs_variables_set_property(JSContext *cx, JSValueConst obj,
ngx_http_variable_t *v;
ngx_http_variable_value_t *vv;
ngx_http_core_main_conf_t *cmcf;
+ u_char storage[64];
r = JS_GetOpaque(obj, NGX_QJS_CLASS_ID_HTTP_VARS);
@@ -6231,11 +6276,22 @@ ngx_http_qjs_variables_set_property(JSContext *cx, JSValueConst obj,
name.len = ngx_strlen(name.data);
- key = ngx_hash_strlow(name.data, name.data, name.len);
+ if (name.len < sizeof(storage)) {
+ lowcase_key = storage;
+
+ } else {
+ lowcase_key = ngx_pnalloc(r->pool, name.len);
+ if (lowcase_key == NULL) {
+ (void) JS_ThrowOutOfMemory(cx);
+ return -1;
+ }
+ }
+
+ key = ngx_hash_strlow(lowcase_key, name.data, name.len);
cmcf = ngx_http_get_module_main_conf(r, ngx_http_core_module);
- v = ngx_hash_find(&cmcf->variables_hash, key, name.data, name.len);
+ v = ngx_hash_find(&cmcf->variables_hash, key, lowcase_key, name.len);
JS_FreeCString(cx, (char *) name.data);
if (v == NULL) {
@@ -6500,13 +6556,19 @@ ngx_http_qjs_header_in(JSContext *cx, ngx_http_request_t *r, unsigned flags,
ngx_table_elt_t **ph;
ngx_http_header_t *hh;
ngx_http_core_main_conf_t *cmcf;
+ u_char storage[128];
/* look up hashed headers */
- lowcase_key = ngx_pnalloc(r->pool, name->len);
- if (lowcase_key == NULL) {
- (void) JS_ThrowOutOfMemory(cx);
- return -1;
+ if (name->len < sizeof(storage)) {
+ lowcase_key = storage;
+
+ } else {
+ lowcase_key = ngx_pnalloc(r->pool, name->len);
+ if (lowcase_key == NULL) {
+ (void) JS_ThrowOutOfMemory(cx);
+ return -1;
+ }
}
hash = ngx_hash_strlow(lowcase_key, name->data, name->len);
diff --git a/nginx/ngx_stream_js_module.c b/nginx/ngx_stream_js_module.c
index 21f65165..a7a923ad 100644
--- a/nginx/ngx_stream_js_module.c
+++ b/nginx/ngx_stream_js_module.c
@@ -1717,6 +1717,7 @@ ngx_stream_js_session_variables(njs_vm_t *vm, njs_object_prop_t *prop,
ngx_stream_variable_t *v;
ngx_stream_core_main_conf_t *cmcf;
ngx_stream_variable_value_t *vv;
+ u_char storage[64];
rc = njs_vm_prop_name(vm, prop, &val);
if (rc != NJS_OK) {
@@ -1724,11 +1725,21 @@ ngx_stream_js_session_variables(njs_vm_t *vm, njs_object_prop_t *prop,
return NJS_DECLINED;
}
- name.data = val.start;
- name.len = val.length;
-
if (setval == NULL) {
- key = ngx_hash_strlow(name.data, name.data, name.len);
+ if (val.length < sizeof(storage)) {
+ name.data = storage;
+
+ } else {
+ name.data = ngx_pnalloc(s->connection->pool, val.length);
+ if (name.data == NULL) {
+ njs_vm_error(vm, "internal error");
+ return NJS_ERROR;
+ }
+ }
+
+ name.len = val.length;
+
+ key = ngx_hash_strlow(name.data, val.start, val.length);
vv = ngx_stream_get_variable(s, &name, key);
if (vv == NULL || vv->not_found) {
@@ -1742,9 +1753,20 @@ ngx_stream_js_session_variables(njs_vm_t *vm, njs_object_prop_t *prop,
cmcf = ngx_stream_get_module_main_conf(s, ngx_stream_core_module);
- key = ngx_hash_strlow(name.data, name.data, name.len);
+ if (val.length < sizeof(storage)) {
+ name.data = storage;
- v = ngx_hash_find(&cmcf->variables_hash, key, name.data, name.len);
+ } else {
+ name.data = ngx_pnalloc(s->connection->pool, val.length);
+ if (name.data == NULL) {
+ njs_vm_error(vm, "internal error");
+ return NJS_ERROR;
+ }
+ }
+
+ key = ngx_hash_strlow(name.data, val.start, val.length);
+
+ v = ngx_hash_find(&cmcf->variables_hash, key, name.data, val.length);
if (v == NULL) {
njs_vm_error(vm, "variable not found");
@@ -2445,10 +2467,11 @@ ngx_stream_qjs_variables_own_property(JSContext *cx,
JSPropertyDescriptor *pdesc, JSValueConst obj, JSAtom prop)
{
uint32_t buffer_type;
- ngx_str_t name;
+ ngx_str_t name, name_lc;
ngx_uint_t key;
ngx_stream_session_t *s;
ngx_stream_variable_value_t *vv;
+ u_char storage[64];
s = JS_GetOpaque(obj, NGX_QJS_CLASS_ID_STREAM_VARS);
@@ -2467,9 +2490,22 @@ ngx_stream_qjs_variables_own_property(JSContext *cx,
name.len = ngx_strlen(name.data);
- key = ngx_hash_strlow(name.data, name.data, name.len);
+ if (name.len < sizeof(storage)) {
+ name_lc.data = storage;
- vv = ngx_stream_get_variable(s, &name, key);
+ } else {
+ name_lc.data = ngx_pnalloc(s->connection->pool, name.len);
+ if (name_lc.data == NULL) {
+ (void) JS_ThrowOutOfMemory(cx);
+ return -1;
+ }
+ }
+
+ name_lc.len = name.len;
+
+ key = ngx_hash_strlow(name_lc.data, name.data, name.len);
+
+ vv = ngx_stream_get_variable(s, &name_lc, key);
JS_FreeCString(cx, (char *) name.data);
if (vv == NULL || vv->not_found) {
return 0;
@@ -2490,13 +2526,14 @@ static int
ngx_stream_qjs_variables_set_property(JSContext *cx, JSValueConst obj,
JSAtom prop, JSValueConst value, JSValueConst receiver, int flags)
{
- ngx_str_t name, val;
+ ngx_str_t name, name_lc, val;
ngx_uint_t key;
ngx_js_ctx_t *ctx;
ngx_stream_session_t *s;
ngx_stream_variable_t *v;
ngx_stream_variable_value_t *vv;
ngx_stream_core_main_conf_t *cmcf;
+ u_char storage[64];
s = JS_GetOpaque(obj, NGX_QJS_CLASS_ID_STREAM_VARS);
@@ -2514,11 +2551,22 @@ ngx_stream_qjs_variables_set_property(JSContext *cx, JSValueConst obj,
name.len = ngx_strlen(name.data);
- key = ngx_hash_strlow(name.data, name.data, name.len);
+ if (name.len < sizeof(storage)) {
+ name_lc.data = storage;
+
+ } else {
+ name_lc.data = ngx_pnalloc(s->connection->pool, name.len);
+ if (name_lc.data == NULL) {
+ (void) JS_ThrowOutOfMemory(cx);
+ return -1;
+ }
+ }
+
+ key = ngx_hash_strlow(name_lc.data, name.data, name.len);
cmcf = ngx_stream_get_module_main_conf(s, ngx_stream_core_module);
- v = ngx_hash_find(&cmcf->variables_hash, key, name.data, name.len);
+ v = ngx_hash_find(&cmcf->variables_hash, key, name_lc.data, name.len);
JS_FreeCString(cx, (char *) name.data);
if (v == NULL) {
diff --git a/nginx/t/js_headers.t b/nginx/t/js_headers.t
index 2cb8c660..8030a4fd 100644
--- a/nginx/t/js_headers.t
+++ b/nginx/t/js_headers.t
@@ -116,6 +116,10 @@ http {
return 200 $test_ifoo_in;
}
+ location /in_lowkey {
+ js_content test.in_lowkey;
+ }
+
location /hdr_in {
js_content test.hdr_in;
}
@@ -355,6 +359,12 @@ $t->write_file('test.js', <<EOF);
return s;
}
+ function in_lowkey(r) {
+ const name = 'X'.repeat(16);
+ let v = r.headersIn[name];
+ r.return(200, name);
+ }
+
function hdr_out(r) {
r.status = 200;
r.headersOut['Foo'] = r.args.foo;
@@ -466,12 +476,12 @@ $t->write_file('test.js', <<EOF);
hdr_out, raw_hdr_out, hdr_out_array, hdr_out_single,
hdr_out_set_cookie, ihdr_out, hdr_out_special_set,
copy_subrequest_hdrs, subrequest, date, last_modified,
- location, location_sr, server};
+ location, location_sr, server, in_lowkey};
EOF
-$t->try_run('no njs')->plan(49);
+$t->try_run('no njs')->plan(50);
###############################################################################
@@ -572,6 +582,8 @@ like(http(
. 'Host: localhost' . CRLF . CRLF
), qr/foo: bar1,\s?bar2/, 'r.headersIn duplicate generic');
+like(http_get('/in_lowkey'), qr/X{16}/, 'r.headersIn name is not overwritten');
+
like(http(
'GET /raw_hdr_in?filter=foo HTTP/1.0' . CRLF
. 'foo: bar1' . CRLF
diff --git a/nginx/t/js_variables.t b/nginx/t/js_variables.t
index f2481e0b..6f1eb173 100644
--- a/nginx/t/js_variables.t
+++ b/nginx/t/js_variables.t
@@ -44,6 +44,7 @@ http {
server_name localhost;
set $foo test.foo_orig;
+ set $XXXXXXXXXXXXXXXX 1;
location /var_set {
return 200 $test_var$foo;
@@ -56,6 +57,10 @@ http {
location /not_found_set {
js_content test.not_found_set;
}
+
+ location /variable_lowkey {
+ js_content test.variable_lowkey;
+ }
}
}
@@ -80,16 +85,33 @@ $t->write_file('test.js', <<EOF);
}
}
- export default {variable, content_set, not_found_set};
+ function variable_lowkey(r) {
+ const name = 'X'.repeat(16);
+
+ if (r.args.set) {
+ r.variables[name] = "1";
+
+ } else {
+ let v = r.variables[name];
+ }
+
+ r.return(200, name);
+ }
+
+ export default {variable, content_set, not_found_set, variable_lowkey};
EOF
-$t->try_run('no njs')->plan(3);
+$t->try_run('no njs')->plan(5);
###############################################################################
like(http_get('/var_set?a=bar'), qr/test_varbar/, 'var set');
like(http_get('/content_set?a=bar'), qr/bar/, 'content set');
like(http_get('/not_found_set'), qr/variable not found/, 'not found exception');
+like(http_get('/variable_lowkey'), qr/X{16}/,
+ 'variable name is not overwritten while reading');
+like(http_get('/variable_lowkey?set=1'), qr/X{16}/,
+ 'variable name is not overwritten while setting');
###############################################################################
diff --git a/nginx/t/stream_js_var.t b/nginx/t/stream_js_var.t
index cde2dc9f..2ab02e2c 100644
--- a/nginx/t/stream_js_var.t
+++ b/nginx/t/stream_js_var.t
@@ -39,8 +39,11 @@ stream {
js_import test.js;
js_var $foo;
+ js_var $XXXXXXXXXXXXXXXX;
js_var $bar a:$remote_addr;
js_set $var test.varr;
+ js_set $lowkey test.lowkey;
+ js_set $lowkey_set test.lowkey_set;
server {
listen 127.0.0.1:8081;
@@ -51,6 +54,16 @@ stream {
listen 127.0.0.1:8082;
return $var$foo;
}
+
+ server {
+ listen 127.0.0.1:8083;
+ return $lowkey;
+ }
+
+ server {
+ listen 127.0.0.1:8084;
+ return $lowkey_set;
+ }
}
EOF
@@ -61,15 +74,31 @@ $t->write_file('test.js', <<EOF);
return '';
}
- export default {varr};
+ function lowkey(s) {
+ const name = 'X'.repeat(16);
+ let v = s.variables[name];
+ return name;
+ }
+
+ function lowkey_set(s) {
+ const name = 'X'.repeat(16);
+ s.variables[name] = 1;
+ return name;
+ }
+
+ export default {varr, lowkey, lowkey_set};
EOF
-$t->try_run('no stream js_var')->plan(2);
+$t->try_run('no stream js_var')->plan(4);
###############################################################################
is(stream('127.0.0.1:' . port(8081))->io('###'), 'a:127.0.0.1',
'default value');
is(stream('127.0.0.1:' . port(8082))->io('###'), 'xxx', 'value set');
+is(stream('127.0.0.1:' . port(8083))->io('###'), 'XXXXXXXXXXXXXXXX',
+ 'variable name is not overwritten while reading');
+is(stream('127.0.0.1:' . port(8084))->io('###'), 'XXXXXXXXXXXXXXXX',
+ 'variable name is not overwritten while writing');
###############################################################################
More information about the nginx-devel
mailing list