[njs] HTTP: validating URI and args arguments in r.subrequest().

[Dmitry Volyntsev]

details:   https://hg.nginx.org/njs/rev/2d6d9d1e8b8f
branches:  
changeset: 2308:2d6d9d1e8b8f
user:      Dmitry Volyntsev <xeioex at nginx.com>
date:      Tue Apr 02 08:52:56 2024 -0700
description:
HTTP: validating URI and args arguments in r.subrequest().

diffstat:

 nginx/ngx_http_js_module.c |  37 +++++++++++++++++++++++++++++++++++++
 nginx/t/js_subrequests.t   |  40 ++++++++++++++++++++++++++++++++++++++--
 2 files changed, 75 insertions(+), 2 deletions(-)

diffs (138 lines):

diff -r 17af51d42ad9 -r 2d6d9d1e8b8f nginx/ngx_http_js_module.c
--- a/nginx/ngx_http_js_module.c	Mon Apr 01 23:13:25 2024 -0700
+++ b/nginx/ngx_http_js_module.c	Tue Apr 02 08:52:56 2024 -0700
@@ -305,6 +305,9 @@ static char *ngx_http_js_merge_loc_conf(
 static ngx_ssl_t *ngx_http_js_ssl(njs_vm_t *vm, ngx_http_request_t *r);
 static ngx_flag_t ngx_http_js_ssl_verify(njs_vm_t *vm, ngx_http_request_t *r);
 
+static ngx_int_t ngx_http_js_parse_unsafe_uri(ngx_http_request_t *r,
+    njs_str_t *uri, njs_str_t *args);
+
 #if (NGX_HTTP_SSL)
 
 static ngx_conf_bitmask_t  ngx_http_js_ssl_protocols[] = {
@@ -3272,6 +3275,11 @@ ngx_http_js_ext_subrequest(njs_vm_t *vm,
         }
     }
 
+    if (ngx_http_js_parse_unsafe_uri(r, &uri_arg, &args_arg) != NGX_OK) {
+        njs_vm_error(vm, "unsafe uri");
+        return NJS_ERROR;
+    }
+
     arg = njs_arg(args, nargs, 3);
 
     if (callback == NULL && !njs_value_is_undefined(arg)) {
@@ -4979,3 +4987,32 @@ ngx_http_js_ssl_verify(njs_vm_t *vm, ngx
     return 0;
 #endif
 }
+
+
+static ngx_int_t
+ngx_http_js_parse_unsafe_uri(ngx_http_request_t *r, njs_str_t *uri,
+    njs_str_t *args)
+{
+    ngx_str_t   uri_arg, args_arg;
+    ngx_uint_t  flags;
+
+    flags = NGX_HTTP_LOG_UNSAFE;
+
+    uri_arg.data = uri->start;
+    uri_arg.len = uri->length;
+
+    args_arg.data = args->start;
+    args_arg.len = args->length;
+
+    if (ngx_http_parse_unsafe_uri(r, &uri_arg, &args_arg, &flags) != NGX_OK) {
+        return NGX_ERROR;
+    }
+
+    uri->start = uri_arg.data;
+    uri->length = uri_arg.len;
+
+    args->start = args_arg.data;
+    args->length = args_arg.len;
+
+    return NGX_OK;
+}
diff -r 17af51d42ad9 -r 2d6d9d1e8b8f nginx/t/js_subrequests.t
--- a/nginx/t/js_subrequests.t	Mon Apr 01 23:13:25 2024 -0700
+++ b/nginx/t/js_subrequests.t	Tue Apr 02 08:52:56 2024 -0700
@@ -148,6 +148,10 @@ http {
             js_content test.sr_unavail_pr;
         }
 
+        location /sr_unsafe {
+            js_content test.sr_unsafe;
+        }
+
         location /sr_broken {
             js_content test.sr_broken;
         }
@@ -385,6 +389,11 @@ EOF
         subrequest_fn_pr(req, ['/unavail'], ['uri', 'status']);
     }
 
+    function sr_unsafe(r) {
+        r.subrequest('../dir');
+        r.return(200);
+    }
+
     function sr_broken(r) {
         r.subrequest('/daemon/unfinished', reply => {
             r.return(200, JSON.stringify({code:reply.status}));
@@ -498,13 +507,14 @@ EOF
                     sr_unavail_pr, sr_broken, sr_too_large, sr_in_sr,
                     sr_js_in_subrequest, sr_js_in_subrequest_pr, js_sub,
                     sr_in_sr_callback, sr_out_of_order, sr_except_not_a_func,
-                    sr_uri_except, sr_except_failed_to_convert_options_arg};
+                    sr_uri_except, sr_except_failed_to_convert_options_arg,
+                    sr_unsafe};
 
 EOF
 
 $t->write_file('t', '["SEE-THIS"]');
 
-$t->try_run('no njs available')->plan(32);
+$t->try_run('no njs available')->plan(33);
 $t->run_daemon(\&http_daemon);
 
 ###############################################################################
@@ -558,6 +568,13 @@ is(get_json('/sr_in_sr_callback'),
 	'{"e":"subrequest can only be created for the primary request"}',
 	'subrequest for non-primary request');
 
+TODO: {
+local $TODO = 'not yet' unless has_version('0.8.4');
+
+like(http_get('/sr_unsafe'), qr/500/s, 'unsafe subrequest uri');
+
+}
+
 $t->stop();
 
 ok(index($t->read_file('error.log'), 'callback is not a function') > 0,
@@ -634,3 +651,22 @@ sub http_daemon {
 }
 
 ###############################################################################
+
+sub has_version {
+	my $need = shift;
+
+	http_get('/njs') =~ /^([.0-9]+)$/m;
+
+	my @v = split(/\./, $1);
+	my ($n, $v);
+
+	for $n (split(/\./, $need)) {
+		$v = shift @v || 0;
+		return 0 if $n > $v;
+		return 1 if $v > $n;
+	}
+
+	return 1;
+}
+
+###############################################################################