HTTP: fix missing validation of the r.return() redirect target

Previously, for a redirect status ngx_http_send_response() stores the
text passed to r.return() as the Location header value, but the text was
only checked when it was used as a response body, where control
characters are valid.

The text is now validated as a header value for the statuses that turn
it into Location, and is left as is otherwise.

diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
index 0f4e98e22a31ffd2d8b7cb9bbd8e8d9c271e9360..cfb250773b7813ab2dd597b7efe3dec4eed9ef02 100644 (file)
--- a/nginx/ngx_http_js_module.c
+++ b/nginx/ngx_http_js_module.c
@@ -536,6 +536,7 @@ static void *ngx_http_js_create_loc_conf(ngx_conf_t *cf);
 static char *ngx_http_js_merge_loc_conf(ngx_conf_t *cf, void *parent,
     void *child);
 
+static ngx_int_t ngx_http_js_status_is_redirect(ngx_int_t status);
 static ngx_int_t ngx_http_js_parse_unsafe_uri(ngx_http_request_t *r,
     njs_str_t *uri, njs_str_t *args);
 
@@ -3276,7 +3277,15 @@ ngx_http_js_ext_return(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
     if (status < NGX_HTTP_BAD_REQUEST
         || !njs_value_is_null_or_undefined(njs_arg(args, nargs, 2)))
     {
-        if (ngx_js_string(vm, njs_arg(args, nargs, 2), &text) != NGX_OK) {
+        if (ngx_http_js_status_is_redirect(status)) {
+            if (ngx_js_header_value(vm, njs_arg(args, nargs, 2), &text)
+                != NGX_OK)
+            {
+                return NJS_ERROR;
+            }
+
+        } else if (ngx_js_string(vm, njs_arg(args, nargs, 2), &text) != NGX_OK)
+        {
             njs_vm_memory_error(vm);
             return NJS_ERROR;
         }
@@ -7201,7 +7210,12 @@ ngx_http_qjs_ext_return(JSContext *cx, JSValueConst this_val,
     ctx = ngx_http_get_module_ctx(r, ngx_http_js_module);
 
     if (status < NGX_HTTP_BAD_REQUEST || !JS_IsNullOrUndefined(argv[1])) {
-        if (ngx_qjs_string(cx, r->pool, argv[1], &body) != NGX_OK) {
+        if (ngx_http_js_status_is_redirect(status)) {
+            if (ngx_qjs_header_value(cx, r->pool, argv[1], &body) != NGX_OK) {
+                return JS_EXCEPTION;
+            }
+
+        } else if (ngx_qjs_string(cx, r->pool, argv[1], &body) != NGX_OK) {
             return JS_ThrowOutOfMemory(cx);
         }
 
@@ -10166,6 +10180,22 @@ ngx_http_js_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child)
 }
 
 
+/*
+ * ngx_http_send_response() turns the text into the Location header
+ * for these statuses, so the text is a header value there.
+ */
+
+static ngx_int_t
+ngx_http_js_status_is_redirect(ngx_int_t status)
+{
+    return status == NGX_HTTP_MOVED_PERMANENTLY
+           || status == NGX_HTTP_MOVED_TEMPORARILY
+           || status == NGX_HTTP_SEE_OTHER
+           || status == NGX_HTTP_TEMPORARY_REDIRECT
+           || status == NGX_HTTP_PERMANENT_REDIRECT;
+}
+
+
 static ngx_int_t
 ngx_http_js_parse_unsafe_uri(ngx_http_request_t *r, njs_str_t *uri,
     njs_str_t *args)

diff --git a/nginx/t/js.t b/nginx/t/js.t
index a8bdab96a5e82f2bf28eb8e64da579f707595dff..7d2d205dbdf15f60ec3a650d8576c5fc7f752ce3 100644 (file)
--- a/nginx/t/js.t
+++ b/nginx/t/js.t
@@ -244,7 +244,7 @@ $t->write_file('test.js', <<EOF);
 
 EOF
 
-$t->try_run('no njs available')->plan(25);
+$t->try_run('no njs available')->plan(28);
 
 ###############################################################################
 
@@ -271,6 +271,12 @@ like(http_get('/return_method?c=200&t=SEE-THIS'), qr/200 OK.*^SEE-THIS$/ms,
        'return text');
 like(http_get('/return_method?c=301&t=path'), qr/ 301 .*Location: path/s,
        'return redirect');
+like(http_get('/return_method?c=301&t=p%0d%0aInjected:%201'), qr/ 500 /,
+       'return invalid redirect');
+unlike(http_get('/return_method?c=301&t=p%0d%0aInjected:%201'), qr/^Injected/m,
+       'return no response splitting');
+like(http_get('/return_method?c=200&t=a%0d%0ab'), qr/200 OK.*^a\r?$/ms,
+       'return body with control characters');
 like(http_get('/return_method?c=404'), qr/404 Not.*html/s, 'return error page');
 like(http_get('/return_method?c=inv'), qr/ 500 /, 'return invalid');