From: Maxim Dounin Date: Fri, 19 Jun 2026 19:54:27 +0000 (+0300) Subject: Script: buffer overrun protection. X-Git-Tag: release-1.31.3~6 X-Git-Url: http://git.kaiwu.me/http/$%7BGITURL%7D/doc/static/gitweb.js?a=commitdiff_plain;h=b767540492e8c79a58bc26034d3bab2f708b7bd1;p=nginx.git Script: buffer overrun protection. With this change, all script copy operations now check if there is enough room in the buffer. To do so, the script engine now provides the e->end pointer, which specifies expected buffer end, and each copy operation is checked against it with the ngx_http_script_check_length() function. The e->end pointer is optional and only checked when set, thus introducing no incompatible API changes. All standard functions were updated to use it, notably ngx_http_complex_value(), ngx_http_script_run(), ngx_http_script_regex_start_code(), ngx_http_script_complex_value_code(). Direct script evaluation in the proxy, fastcgi, scgi, uwsgi, grpc proxy, index, and try_files modules will be updated by a separate patch. In particular, this catches issues as observed when evaluating variables with side effects, such as in the following configuration: map $uri $map { ~(?.*) $capture; } set $capture ""; set $temp "$capture $map"; As well as when evaluating non-cacheable variables, where length of a variable might change between length and copy codes, such as in the following configuration: map prefix:$capture $map_volatile { volatile; ~(?.*) $capture; } set $capture ""; set $temp "$map_volatile"; Similar changes were made in the stream module. Signed-off-by: Roman Arutyunyan Origin: --- diff --git a/src/http/ngx_http_script.c b/src/http/ngx_http_script.c index 8a7ba57e2..c7e28b231 100644 --- a/src/http/ngx_http_script.c +++ b/src/http/ngx_http_script.c @@ -92,12 +92,17 @@ ngx_http_complex_value(ngx_http_request_t *r, ngx_http_complex_value_t *val, e.ip = val->values; e.pos = value->data; e.buf = *value; + e.end = value->data + len; while (*(uintptr_t *) e.ip) { code = *(ngx_http_script_code_pt *) e.ip; code((ngx_http_script_engine_t *) &e); } + if (e.status) { + return NGX_ERROR; + } + *value = e.buf; return NGX_OK; @@ -652,12 +657,17 @@ ngx_http_script_run(ngx_http_request_t *r, ngx_str_t *value, e.ip = code_values; e.pos = value->data; + e.end = value->data + len; while (*(uintptr_t *) e.ip) { code = *(ngx_http_script_code_pt *) e.ip; code((ngx_http_script_engine_t *) &e); } + if (e.status) { + return NULL; + } + return e.pos; } @@ -807,6 +817,25 @@ ngx_http_script_add_code(ngx_array_t *codes, size_t size, void *code) } +ngx_int_t +ngx_http_script_check_length(ngx_http_script_engine_t *e, size_t len) +{ + if (e->end == NULL) { + return NGX_OK; + } + + if (e->end - e->pos < (ssize_t) len) { + ngx_log_error(NGX_LOG_ALERT, e->request->connection->log, 0, + "no buffer space in script copy"); + e->ip = ngx_http_script_exit; + e->status = NGX_HTTP_INTERNAL_SERVER_ERROR; + return NGX_ERROR; + } + + return NGX_OK; +} + + static ngx_int_t ngx_http_script_add_copy_code(ngx_http_script_compile_t *sc, ngx_str_t *value, ngx_uint_t last) @@ -875,6 +904,11 @@ ngx_http_script_copy_code(ngx_http_script_engine_t *e) p = e->pos; if (!e->skip) { + + if (ngx_http_script_check_length(e, code->len) != NGX_OK) { + return; + } + e->pos = ngx_copy(p, e->ip + sizeof(ngx_http_script_copy_code_t), code->len); } @@ -978,6 +1012,11 @@ ngx_http_script_copy_var_code(ngx_http_script_engine_t *e) } if (value && !value->not_found) { + + if (ngx_http_script_check_length(e, value->len) != NGX_OK) { + return; + } + p = e->pos; e->pos = ngx_copy(p, value->data, value->len); @@ -1195,6 +1234,7 @@ ngx_http_script_regex_start_code(ngx_http_script_engine_t *e) e->quote = code->redirect; e->pos = e->buf.data; + e->end = e->buf.data + e->buf.len; e->ip += sizeof(ngx_http_script_regex_code_t); } @@ -1232,6 +1272,11 @@ ngx_http_script_regex_end_code(ngx_http_script_engine_t *e) e->pos = dst; if (code->add_args && r->args.len) { + + if (ngx_http_script_check_length(e, r->args.len + 1) != NGX_OK) { + return; + } + *e->pos++ = (u_char) (code->args ? '&' : '?'); e->pos = ngx_copy(e->pos, r->args.data, r->args.len); } @@ -1265,6 +1310,11 @@ ngx_http_script_regex_end_code(ngx_http_script_engine_t *e) e->buf.len = e->args - e->buf.data; if (code->add_args && r->args.len) { + + if (ngx_http_script_check_length(e, r->args.len + 1) != NGX_OK) { + return; + } + *e->pos++ = '&'; e->pos = ngx_copy(e->pos, r->args.data, r->args.len); } @@ -1386,6 +1436,7 @@ ngx_http_script_copy_capture_code(ngx_http_script_engine_t *e) int *cap; u_char *p, *pos; size_t len; + uintptr_t escape; ngx_uint_t n; ngx_http_request_t *r; ngx_http_script_copy_capture_code_t *code; @@ -1409,9 +1460,20 @@ ngx_http_script_copy_capture_code(ngx_http_script_engine_t *e) if ((e->is_args || e->quote) && (e->request->quoted_uri || e->request->plus_in_uri)) { + escape = 2 * ngx_escape_uri(NULL, p, len, NGX_ESCAPE_ARGS); + + if (ngx_http_script_check_length(e, len + escape) != NGX_OK) { + return; + } + e->pos = (u_char *) ngx_escape_uri(pos, p, len, NGX_ESCAPE_ARGS); } else { + + if (ngx_http_script_check_length(e, len) != NGX_OK) { + return; + } + e->pos = ngx_copy(pos, p, len); } } @@ -1796,6 +1858,7 @@ ngx_http_script_complex_value_code(ngx_http_script_engine_t *e) } e->pos = e->buf.data; + e->end = e->buf.data + len; e->sp->len = e->buf.len; e->sp->data = e->buf.data; diff --git a/src/http/ngx_http_script.h b/src/http/ngx_http_script.h index 43600383c..b086b266f 100644 --- a/src/http/ngx_http_script.h +++ b/src/http/ngx_http_script.h @@ -17,6 +17,7 @@ typedef struct { u_char *ip; u_char *pos; + u_char *end; ngx_http_variable_value_t *sp; ngx_str_t buf; @@ -240,6 +241,9 @@ void *ngx_http_script_start_code(ngx_pool_t *pool, ngx_array_t **codes, size_t size); void *ngx_http_script_add_code(ngx_array_t *codes, size_t size, void *code); +ngx_int_t ngx_http_script_check_length(ngx_http_script_engine_t *e, + size_t len); + size_t ngx_http_script_copy_len_code(ngx_http_script_engine_t *e); void ngx_http_script_copy_code(ngx_http_script_engine_t *e); size_t ngx_http_script_copy_var_len_code(ngx_http_script_engine_t *e); diff --git a/src/stream/ngx_stream_script.c b/src/stream/ngx_stream_script.c index d1f256caf..431364b49 100644 --- a/src/stream/ngx_stream_script.c +++ b/src/stream/ngx_stream_script.c @@ -92,6 +92,7 @@ ngx_stream_complex_value(ngx_stream_session_t *s, e.ip = val->values; e.pos = value->data; + e.end = value->data + len; e.buf = *value; while (*(uintptr_t *) e.ip) { @@ -99,6 +100,10 @@ ngx_stream_complex_value(ngx_stream_session_t *s, code((ngx_stream_script_engine_t *) &e); } + if (e.status) { + return NGX_ERROR; + } + *value = e.buf; return NGX_OK; @@ -528,12 +533,17 @@ ngx_stream_script_run(ngx_stream_session_t *s, ngx_str_t *value, e.ip = code_values; e.pos = value->data; + e.end = value->data + len; while (*(uintptr_t *) e.ip) { code = *(ngx_stream_script_code_pt *) e.ip; code((ngx_stream_script_engine_t *) &e); } + if (e.status) { + return NULL; + } + return e.pos; } @@ -670,6 +680,25 @@ ngx_stream_script_add_code(ngx_array_t *codes, size_t size, void *code) } +ngx_int_t +ngx_stream_script_check_length(ngx_stream_script_engine_t *e, size_t len) +{ + if (e->end == NULL) { + return NGX_OK; + } + + if (e->end - e->pos < (ssize_t) len) { + ngx_log_error(NGX_LOG_ALERT, e->session->connection->log, 0, + "no buffer space in script copy"); + e->ip = ngx_stream_script_exit; + e->status = NGX_STREAM_INTERNAL_SERVER_ERROR; + return NGX_ERROR; + } + + return NGX_OK; +} + + static ngx_int_t ngx_stream_script_add_copy_code(ngx_stream_script_compile_t *sc, ngx_str_t *value, ngx_uint_t last) @@ -739,6 +768,11 @@ ngx_stream_script_copy_code(ngx_stream_script_engine_t *e) p = e->pos; if (!e->skip) { + + if (ngx_stream_script_check_length(e, code->len) != NGX_OK) { + return; + } + e->pos = ngx_copy(p, e->ip + sizeof(ngx_stream_script_copy_code_t), code->len); } @@ -843,6 +877,11 @@ ngx_stream_script_copy_var_code(ngx_stream_script_engine_t *e) } if (value && !value->not_found) { + + if (ngx_stream_script_check_length(e, value->len) != NGX_OK) { + return; + } + p = e->pos; e->pos = ngx_copy(p, value->data, value->len); @@ -943,6 +982,11 @@ ngx_stream_script_copy_capture_code(ngx_stream_script_engine_t *e) cap = s->captures; len = cap[n + 1] - cap[n]; p = s->captures_data + cap[n]; + + if (ngx_stream_script_check_length(e, len) != NGX_OK) { + return; + } + e->pos = ngx_copy(pos, p, len); } @@ -1015,6 +1059,7 @@ ngx_stream_script_full_name_code(ngx_stream_script_engine_t *e) != NGX_OK) { e->ip = ngx_stream_script_exit; + e->status = NGX_STREAM_INTERNAL_SERVER_ERROR; return; } diff --git a/src/stream/ngx_stream_script.h b/src/stream/ngx_stream_script.h index d8f374047..dc6de9c72 100644 --- a/src/stream/ngx_stream_script.h +++ b/src/stream/ngx_stream_script.h @@ -17,6 +17,7 @@ typedef struct { u_char *ip; u_char *pos; + u_char *end; ngx_stream_variable_value_t *sp; ngx_str_t buf; @@ -25,6 +26,7 @@ typedef struct { unsigned flushed:1; unsigned skip:1; + ngx_int_t status; ngx_stream_session_t *session; } ngx_stream_script_engine_t; @@ -127,6 +129,9 @@ void ngx_stream_script_flush_no_cacheable_variables(ngx_stream_session_t *s, void *ngx_stream_script_add_code(ngx_array_t *codes, size_t size, void *code); +ngx_int_t ngx_stream_script_check_length(ngx_stream_script_engine_t *e, + size_t len); + size_t ngx_stream_script_copy_len_code(ngx_stream_script_engine_t *e); void ngx_stream_script_copy_code(ngx_stream_script_engine_t *e); size_t ngx_stream_script_copy_var_len_code(ngx_stream_script_engine_t *e);