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.

diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
index 66cb97c0583510bccf8638a8c01c2f57a0da0c77..7cd874019f204578e1f3a34fc7ebcd0b2c69754e 100644 (file)
--- 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 21f65165e8cc62e34398bd360726e99813ef9ffd..a7a923ad256a7f3f7daf65a5ca6753d7d9ebea3d 100644 (file)
--- 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 2cb8c66094a98ff671bb026445a0256ede08a1aa..8030a4fd1d2630edaffcc18c5e2d12d956911eb6 100644 (file)
--- 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 f2481e0ba83c7ed6c162e294a0c4938a14187447..6f1eb1735f4d9021745eb35c1d4443ee482eedc7 100644 (file)
--- 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 cde2dc9f4cc3ebb0d8c7d1678fba425f0770de52..2ab02e2caebbea467483d5fe8d6052db11ac5660 100644 (file)
--- 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');
 
 ###############################################################################