HTTP: fix missing validation of r.headersOut values

Previously, a property name used to store a response header was copied
into the header list as is, without checking it against the token
grammar, so a name containing control characters produced a malformed
response.

The name is now validated in the two places where it is resolved for a
store: ngx_http_js_ext_header_out() and its QuickJS counterpart, which
also covers Object.defineProperty().  An invalid name is rejected with a
TypeError.

Previously, values assigned to r.headersOut were stored as is, without
checking them for control characters, while ngx_js_check_header_value()
was applied only on the Fetch Headers path.

The conversion of a JS value into a header value is now done by
ngx_js_header_value() and ngx_qjs_header_value(), which validate the
result.  All the r.headersOut store paths, including the special headers
and Content-Type, use them and reject an invalid value with a TypeError.

Reported by vladimirelitokarev@gmail.com.

diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
index efb0220f67fe623dfd11e788ce434efb8321ab4a..0f4e98e22a31ffd2d8b7cb9bbd8e8d9c271e9360 100644 (file)
--- a/nginx/ngx_http_js_module.c
+++ b/nginx/ngx_http_js_module.c
@@ -10,6 +10,7 @@
 #include <ngx_core.h>
 #include <ngx_http.h>
 #include "ngx_js.h"
+#include "ngx_js_http.h"
 #include "ngx_js_modules.h"
 #include "ngx_js_form.h"
 
@@ -2463,6 +2464,14 @@ ngx_http_js_ext_header_out(njs_vm_t *vm, njs_object_prop_t *prop,
         return NJS_DECLINED;
     }
 
+    if (setval != NULL
+        && ngx_js_check_header_name(name.start, name.length) != NGX_OK)
+    {
+        njs_vm_type_error(vm, "%s",
+                          ngx_js_headers_error(NGX_JS_HEADERS_INVALID_NAME));
+        return NJS_ERROR;
+    }
+
     if (r->header_sent && setval != NULL) {
         ngx_log_error(NGX_LOG_WARN, r->connection->log, 0,
                       "ignored setting of response header \"%V\" because"
@@ -2544,7 +2553,7 @@ ngx_http_js_header_out_special(njs_vm_t *vm, ngx_http_request_t *r,
         setval = njs_vm_array_prop(vm, setval, length - 1, &lvalue);
     }
 
-    if (ngx_js_string(vm, setval, &s) != NGX_OK) {
+    if (ngx_js_header_value(vm, setval, &s) != NGX_OK) {
         return NJS_ERROR;
     }
 
@@ -2790,7 +2799,7 @@ ngx_http_js_header_generic(njs_vm_t *vm, ngx_http_request_t *r,
             setval = njs_vm_array_prop(vm, array, i, &lvalue);
         }
 
-        if (ngx_js_string(vm, setval, &s) != NGX_OK) {
+        if (ngx_js_header_value(vm, setval, &s) != NGX_OK) {
             return NJS_ERROR;
         }
 
@@ -5239,7 +5248,7 @@ ngx_http_js_header_out(njs_vm_t *vm, ngx_http_request_t *r, unsigned flags,
             setval = njs_vm_array_prop(vm, array, i, &lvalue);
         }
 
-        if (ngx_js_string(vm, setval, &s) != NGX_OK) {
+        if (ngx_js_header_value(vm, setval, &s) != NGX_OK) {
             return NJS_ERROR;
         }
 
@@ -5319,7 +5328,7 @@ ngx_http_js_header_out_special(njs_vm_t *vm, ngx_http_request_t *r,
         setval = njs_vm_array_prop(vm, setval, length - 1, &lvalue);
     }
 
-    if (ngx_js_string(vm, setval, &s) != NGX_OK) {
+    if (ngx_js_header_value(vm, setval, &s) != NGX_OK) {
         return NJS_ERROR;
     }
 
@@ -5594,7 +5603,7 @@ ngx_http_js_content_type(njs_vm_t *vm, ngx_http_request_t *r,
         setval = njs_vm_array_prop(vm, setval, length - 1, &lvalue);
     }
 
-    if (ngx_js_string(vm, setval, &s) != NGX_OK) {
+    if (ngx_js_header_value(vm, setval, &s) != NGX_OK) {
         return NJS_ERROR;
     }
 
@@ -8674,7 +8683,7 @@ ngx_http_qjs_headers_out_handler(JSContext *cx, ngx_http_request_t *r,
             }
         }
 
-        rc = ngx_qjs_string(cx, r->pool, v, &s);
+        rc = ngx_qjs_header_value(cx, r->pool, v, &s);
 
         if (qjs_is_array(cx, *value)) {
             JS_FreeValue(cx, v);
@@ -8766,7 +8775,7 @@ ngx_http_qjs_headers_out_special_handler(JSContext *cx, ngx_http_request_t *r,
         setval = JS_UNDEFINED;
     }
 
-    rc = ngx_qjs_string(cx, r->pool, setval, &s);
+    rc = ngx_qjs_header_value(cx, r->pool, setval, &s);
 
     if (value != NULL && qjs_is_array(cx, *value)) {
         JS_FreeValue(cx, setval);
@@ -8992,7 +9001,7 @@ ngx_http_qjs_headers_out_content_type(JSContext *cx, ngx_http_request_t *r,
         setval = *value;
     }
 
-    rc = ngx_qjs_string(cx, r->pool, setval, &s);
+    rc = ngx_qjs_header_value(cx, r->pool, setval, &s);
 
     if (qjs_is_array(cx, *value)) {
         JS_FreeValue(cx, setval);
@@ -9204,6 +9213,13 @@ ngx_http_qjs_headers_out_define_own_property(JSContext *cx,
 
     name.len = ngx_strlen(name.data);
 
+    if (ngx_js_check_header_name(name.data, name.len) != NGX_OK) {
+        JS_FreeCString(cx, (char *) name.data);
+        (void) JS_ThrowTypeError(cx, "%s",
+                            ngx_js_headers_error(NGX_JS_HEADERS_INVALID_NAME));
+        return -1;
+    }
+
     if (r->header_sent) {
         ngx_log_error(NGX_LOG_WARN, r->connection->log, 0,
                       "ignored setting of response header \"%V\" because"

diff --git a/nginx/ngx_js.c b/nginx/ngx_js.c
index 3b69cea5c855516c010edf2d287bd1230c480d9e..c077f50368be32a84af1660f1eebcda08d057bc9 100644 (file)
--- a/nginx/ngx_js.c
+++ b/nginx/ngx_js.c
@@ -1638,6 +1638,24 @@ string:
 }
 
 
+ngx_int_t
+ngx_qjs_header_value(JSContext *cx, ngx_pool_t *pool, JSValueConst val,
+    ngx_str_t *dst)
+{
+    if (ngx_qjs_string(cx, pool, val, dst) != NGX_OK) {
+        return NGX_ERROR;
+    }
+
+    if (ngx_js_check_header_value(dst->data, dst->len) != NGX_OK) {
+        (void) JS_ThrowTypeError(cx, "%s",
+                            ngx_js_headers_error(NGX_JS_HEADERS_INVALID_VALUE));
+        return NGX_ERROR;
+    }
+
+    return NGX_OK;
+}
+
+
 static void
 ngx_qjs_timer_handler(ngx_event_t *ev)
 {
@@ -2520,6 +2538,23 @@ ngx_js_ngx_string(njs_vm_t *vm, njs_value_t *value, ngx_str_t *str)
 }
 
 
+ngx_int_t
+ngx_js_header_value(njs_vm_t *vm, njs_value_t *value, njs_str_t *str)
+{
+    if (ngx_js_string(vm, value, str) != NGX_OK) {
+        return NGX_ERROR;
+    }
+
+    if (ngx_js_check_header_value(str->start, str->length) != NGX_OK) {
+        njs_vm_type_error(vm, "%s",
+                          ngx_js_headers_error(NGX_JS_HEADERS_INVALID_VALUE));
+        return NGX_ERROR;
+    }
+
+    return NGX_OK;
+}
+
+
 static njs_int_t
 njs_function_bind(njs_vm_t *vm, const njs_str_t *name,
     njs_function_native_t native, njs_bool_t ctor)

diff --git a/nginx/ngx_js.h b/nginx/ngx_js.h
index c6cc8a3e7690e55b0ea877ee6b0b81b13caa6af1..5eadfa3230fbd0d891fcc91d9d9ef7b708b8ee3b 100644 (file)
--- a/nginx/ngx_js.h
+++ b/nginx/ngx_js.h
@@ -400,6 +400,8 @@ ngx_int_t ngx_qjs_log_exception(ngx_engine_t *e, ngx_log_t *log,
 ngx_int_t ngx_qjs_integer(JSContext *cx, JSValueConst val, ngx_int_t *n);
 ngx_int_t ngx_qjs_string(JSContext *cx, ngx_pool_t *pool, JSValueConst val,
     ngx_str_t *dst);
+ngx_int_t ngx_qjs_header_value(JSContext *cx, ngx_pool_t *pool,
+    JSValueConst val, ngx_str_t *dst);
 
 JSValue ngx_qjs_ext_fetch(JSContext *cx, JSValueConst this_val, int argc,
      JSValueConst *argv);
@@ -484,6 +486,8 @@ njs_int_t ngx_js_ext_flags(njs_vm_t *vm, njs_object_prop_t *prop, uint32_t unuse
 
 ngx_int_t ngx_js_string(njs_vm_t *vm, njs_value_t *value, njs_str_t *str);
 ngx_int_t ngx_js_ngx_string(njs_vm_t *vm, njs_value_t *value, ngx_str_t *str);
+ngx_int_t ngx_js_header_value(njs_vm_t *vm, njs_value_t *value,
+    njs_str_t *str);
 ngx_int_t ngx_js_integer(njs_vm_t *vm, njs_value_t *value, ngx_int_t *n);
 const char *ngx_js_errno_string(int errnum);
 

diff --git a/nginx/t/js_headers.t b/nginx/t/js_headers.t
index ea8a31c634b731b64b8f17b02f681220cb8bd233..922f75348209dac319aba12c3e688d8414c3deef 100644 (file)
--- a/nginx/t/js_headers.t
+++ b/nginx/t/js_headers.t
@@ -144,6 +144,14 @@ http {
             js_content test.hdr_out_array;
         }
 
+        location /hdr_out_invalid {
+            js_content test.hdr_out_invalid;
+        }
+
+        location /hdr_out_valid {
+            js_content test.hdr_out_valid;
+        }
+
         location /hdr_out_set_cookie {
             js_content test.hdr_out_set_cookie;
         }
@@ -415,6 +423,32 @@ $t->write_file('test.js', <<EOF);
         r.return(200, `B:\${r.headersOut.foo}`);
     }
 
+    function hdr_out_invalid(r) {
+        var v = 'a\\r\\nInjected: 1';
+
+        try {
+            if (r.args.h == 'arr') {
+                r.headersOut['Foo'] = ['good', v];
+
+            } else if (r.args.h == 'name') {
+                r.headersOut['X\\r\\nInjected: 1'] = 'val';
+
+            } else {
+                r.headersOut[r.args.h] = v;
+            }
+
+            r.return(200, 'no_error');
+
+        } catch (e) {
+            r.return(500, `\${e.constructor.name}:\${e.message}`);
+        }
+    }
+
+    function hdr_out_valid(r) {
+        r.headersOut['Foo'] = 'a\\tb\\u00e9';
+        r.return(200, 'ok');
+    }
+
     function hdr_out_single(r) {
         r.headersOut.ETag = ['a', 'b'];
         r.return(200, `B:\${r.headersOut.etag}`);
@@ -489,6 +523,7 @@ $t->write_file('test.js', <<EOF);
                     content_encoding, content_encoding_arr, headers_list,
                     hdr_in, raw_hdr_in, hdr_sorted_keys, foo_in, ifoo_in,
                     hdr_out, raw_hdr_out, hdr_out_array, hdr_out_single,
+                    hdr_out_invalid, hdr_out_valid,
                     hdr_out_set_cookie, ihdr_out, hdr_out_special_set,
                     copy_subrequest_hdrs, subrequest, date, last_modified,
                     location, location_sr, server, in_lowkey};
@@ -496,7 +531,7 @@ $t->write_file('test.js', <<EOF);
 
 EOF
 
-$t->try_run('no njs')->plan(53);
+$t->try_run('no njs')->plan(61);
 
 ###############################################################################
 
@@ -546,6 +581,26 @@ like(http_get('/hdr_out_single'), qr/ETag: a\r\nETag: b/,
        'r.headersOut single');
 like(http_get('/hdr_out_single'), qr/B:a/,
        'r.headersOut single get');
+like(http_get('/hdr_out_invalid?h=Foo'),
+       qr/500.*TypeError:invalid header value/s, 'r.headersOut invalid value');
+like(http_get('/hdr_out_invalid?h=Location'),
+       qr/500.*TypeError:invalid header value/s,
+       'r.headersOut invalid special value');
+like(http_get('/hdr_out_invalid?h=Content-Type'),
+       qr/500.*TypeError:invalid header value/s,
+       'r.headersOut invalid Content-Type value');
+like(http_get('/hdr_out_invalid?h=arr'),
+       qr/500.*TypeError:invalid header value/s,
+       'r.headersOut invalid value in array');
+like(http_get('/hdr_out_invalid?h=name'),
+       qr/500.*TypeError:invalid header name/s, 'r.headersOut invalid name');
+unlike(http_get('/hdr_out_invalid?h=Foo'), qr/^Injected/m,
+       'r.headersOut no response splitting');
+unlike(http_get('/hdr_out_invalid?h=name'), qr/^Injected/m,
+       'r.headersOut no response splitting by name');
+like(http_get('/hdr_out_valid'), qr/Foo: a\tb\xc3\xa9/,
+       'r.headersOut obs-text value');
+
 like(http_get('/hdr_out_set_cookie'), qr/Set-Cookie: c\r\nSet-Cookie: d/,
        'set_cookie');
 like(http_get('/hdr_out_set_cookie'), qr/B:c,d,f true/,