From: Dmitry Volyntsev Date: Thu, 13 Aug 2026 22:14:42 +0000 (-0700) Subject: HTTP: fix missing validation of the r.return() redirect target X-Git-Tag: 1.0.1~12 X-Git-Url: https://git.kaiwu.me/?a=commitdiff_plain;h=2ca3f08c79f4ce1ab6248baf0f00a28824950464;p=njs.git 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 0f4e98e2..cfb25077 100644 --- 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 a8bdab96..7d2d205d 100644 --- a/nginx/t/js.t +++ b/nginx/t/js.t @@ -244,7 +244,7 @@ $t->write_file('test.js', <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');