[PATCH] Upstream: avoid side effects on r->args when creating requests
Maxim Dounin
mdounin at mdounin.ru
Wed Jul 29 04:34:21 UTC 2026
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1785299238 -10800
# Wed Jul 29 07:27:18 2026 +0300
# Node ID ca7413416a812fd5c5dbd77a04d649cd71008bbd
# Parent 1260ebda75742581472b1b105c9af202a5cc04d1
Upstream: avoid side effects on r->args when creating requests.
Previously, request creation functions in proxy and gRPC proxy assumed
that r->args did not change, but it can be changed as a result of
proxy_set_header, proxy_set_body, and grpc_set_header variable
evaluation, potentially resulting in buffer overrun. In particular,
since freenginx 1.31.3 this can be caused by a map where $args is
changed by regular expression named captures, and in older versions this
could happen due to 3rd party modules.
The fix is to copy r->args (and also r->uri, for consistency) into a
local variables when creating a request, so length calculations are
guaranteed to use the same values as when writing the buffer.
Similarly, in gRPC proxy local variables for r->valid_unparsed_uri,
r->args, and r->uri are introduced.
diff --git a/src/http/modules/ngx_http_grpc_module.c b/src/http/modules/ngx_http_grpc_module.c
--- a/src/http/modules/ngx_http_grpc_module.c
+++ b/src/http/modules/ngx_http_grpc_module.c
@@ -716,7 +716,8 @@ ngx_http_grpc_create_request(ngx_http_re
key_len, val_len, uri_len;
uintptr_t escape;
ngx_buf_t *b;
- ngx_uint_t i, next;
+ ngx_str_t uri, args;
+ ngx_uint_t i, next, unparsed_uri;
ngx_chain_t *cl, *body;
ngx_list_part_t *part;
ngx_table_elt_t *header;
@@ -739,6 +740,12 @@ ngx_http_grpc_create_request(ngx_http_re
headers_len = 0;
+#if (NGX_SUPPRESS_WARN)
+ escape = 0;
+ ngx_str_null(&uri);
+ ngx_str_null(&args);
+#endif
+
/* :method header */
if (r->method == NGX_HTTP_GET || r->method == NGX_HTTP_POST) {
@@ -757,13 +764,15 @@ ngx_http_grpc_create_request(ngx_http_re
/* :path header */
if (r->valid_unparsed_uri) {
- escape = 0;
+ unparsed_uri = 1;
uri_len = r->unparsed_uri.len;
} else {
- escape = 2 * ngx_escape_uri(NULL, r->uri.data, r->uri.len,
- NGX_ESCAPE_URI);
- uri_len = r->uri.len + escape + sizeof("?") - 1 + r->args.len;
+ unparsed_uri = 0;
+ uri = r->uri;
+ args = r->args;
+ escape = 2 * ngx_escape_uri(NULL, uri.data, uri.len, NGX_ESCAPE_URI);
+ uri_len = uri.len + escape + sizeof("?") - 1 + args.len;
}
len += 1 + NGX_HTTP_V2_INT_OCTETS + uri_len;
@@ -948,7 +957,7 @@ ngx_http_grpc_create_request(ngx_http_re
"grpc header: \":scheme: http\"");
}
- if (r->valid_unparsed_uri) {
+ if (unparsed_uri) {
if (r->unparsed_uri.len > NGX_HTTP_V2_MAX_FIELD) {
ngx_log_error(NGX_LOG_CRIT, r->connection->log, 0,
@@ -970,20 +979,20 @@ ngx_http_grpc_create_request(ngx_http_re
ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
"grpc header: \":path: %V\"", &r->unparsed_uri);
- } else if (escape || r->args.len > 0) {
+ } else if (escape || args.len > 0) {
p = val_tmp;
if (escape) {
- p = (u_char *) ngx_escape_uri(p, r->uri.data, r->uri.len,
+ p = (u_char *) ngx_escape_uri(p, uri.data, uri.len,
NGX_ESCAPE_URI);
} else {
- p = ngx_copy(p, r->uri.data, r->uri.len);
+ p = ngx_copy(p, uri.data, uri.len);
}
- if (r->args.len > 0) {
+ if (args.len > 0) {
*p++ = '?';
- p = ngx_copy(p, r->args.data, r->args.len);
+ p = ngx_copy(p, args.data, args.len);
}
if (p - val_tmp > NGX_HTTP_V2_MAX_FIELD) {
@@ -1002,20 +1011,19 @@ ngx_http_grpc_create_request(ngx_http_re
} else {
- if (r->uri.len > NGX_HTTP_V2_MAX_FIELD) {
+ if (uri.len > NGX_HTTP_V2_MAX_FIELD) {
ngx_log_error(NGX_LOG_CRIT, r->connection->log, 0,
"too long grpc request header value: "
"\":path: %*s...\"",
- 256, r->uri.data);
+ 256, uri.data);
return NGX_ERROR;
}
*b->last++ = ngx_http_v2_inc_indexed(NGX_HTTP_V2_PATH_INDEX);
- b->last = ngx_http_v2_write_value(b->last, r->uri.data,
- r->uri.len, tmp);
+ b->last = ngx_http_v2_write_value(b->last, uri.data, uri.len, tmp);
ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
- "grpc header: \":path: %V\"", &r->uri);
+ "grpc header: \":path: %V\"", &uri);
}
if (!glcf->host_set) {
diff --git a/src/http/modules/ngx_http_proxy_module.c b/src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -1257,7 +1257,7 @@ ngx_http_proxy_create_request(ngx_http_r
key_len, val_len;
uintptr_t escape;
ngx_buf_t *b;
- ngx_str_t method;
+ ngx_str_t method, uri, args;
ngx_uint_t i, unparsed_uri;
ngx_chain_t *cl, *body;
ngx_list_part_t *part;
@@ -1310,6 +1310,11 @@ ngx_http_proxy_create_request(ngx_http_r
body_len = 0;
headers_len = 0;
+#if (NGX_SUPPRESS_WARN)
+ ngx_str_null(&uri);
+ ngx_str_null(&args);
+#endif
+
if (plcf->proxy_lengths && ctx->vars.uri.len) {
uri_len = ctx->vars.uri.len;
@@ -1321,13 +1326,16 @@ ngx_http_proxy_create_request(ngx_http_r
loc_len = (r->valid_location && ctx->vars.uri.len) ?
plcf->location.len : 0;
+ uri = r->uri;
+ args = r->args;
+
if (r->quoted_uri || r->internal) {
- escape = 2 * ngx_escape_uri(NULL, r->uri.data + loc_len,
- r->uri.len - loc_len, NGX_ESCAPE_URI);
+ escape = 2 * ngx_escape_uri(NULL, uri.data + loc_len,
+ uri.len - loc_len, NGX_ESCAPE_URI);
}
- uri_len = ctx->vars.uri.len + r->uri.len - loc_len + escape
- + sizeof("?") - 1 + r->args.len;
+ uri_len = ctx->vars.uri.len + uri.len - loc_len + escape
+ + sizeof("?") - 1 + args.len;
}
if (uri_len == 0) {
@@ -1452,18 +1460,17 @@ ngx_http_proxy_create_request(ngx_http_r
}
if (escape) {
- ngx_escape_uri(b->last, r->uri.data + loc_len,
- r->uri.len - loc_len, NGX_ESCAPE_URI);
- b->last += r->uri.len - loc_len + escape;
+ ngx_escape_uri(b->last, uri.data + loc_len,
+ uri.len - loc_len, NGX_ESCAPE_URI);
+ b->last += uri.len - loc_len + escape;
} else {
- b->last = ngx_copy(b->last, r->uri.data + loc_len,
- r->uri.len - loc_len);
+ b->last = ngx_copy(b->last, uri.data + loc_len, uri.len - loc_len);
}
- if (r->args.len > 0) {
+ if (args.len > 0) {
*b->last++ = '?';
- b->last = ngx_copy(b->last, r->args.data, r->args.len);
+ b->last = ngx_copy(b->last, args.data, args.len);
}
}
More information about the nginx-devel
mailing list