From 47d3daf0f45df999a9f5466674f963452834622c Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Mon, 10 Mar 2025 18:59:12 -0400 Subject: [PATCH] SECURITY FIX: Proxy: escape ';' in URI paths This fixes a security hole when used with many Java application servers, which treat ';' as a special character in paths. See [1] for more details. This changes how Nginx constructs requests for its backends. Something like: location /fake1/ { proxy_pass http://127.0.0.1:8000/fake2/; } will now escape semicolons in the provided URL. This is ultimately a flaw in Nginx, not the backend servers: it is a client error to make an HTTP request with a URI that contains a path with a character that is not valid in paths, and as the client Nginx is responsible for only making valid requests. This is not a complete fix. A complete fix would require blocking %2F in paths (HAProxy and NGINX disagree on its meaning) and blocking unescaped semicolons in paths (NGINX and Java app servers disagree on their meaning). This does fix the case where proxy_pass is used with a URI that does not contain any variables. [1]: https://i.blackhat.com/us-18/Wed-August-8/us-18-Orange-Tsai-Breaking-Parser-Logic-Take-Your-Path-Normalization-Off-And-Pop-0days-Out-2.pdf --- src/core/ngx_string.c | 23 +++++++++++++++++++-- src/core/ngx_string.h | 1 + src/http/modules/ngx_http_proxy_module.c | 18 ++++++---------- src/http/modules/ngx_http_proxy_v2_module.c | 8 +++---- 4 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/core/ngx_string.c b/src/core/ngx_string.c index 10fe764c3..e35647921 100644 --- a/src/core/ngx_string.c +++ b/src/core/ngx_string.c @@ -1566,6 +1566,26 @@ ngx_escape_uri(u_char *dst, u_char *src, size_t size, ngx_uint_t type) /* ~}| {zyx wvut srqp onml kjih gfed cba` */ 0xb8000001, /* 1011 1000 0000 0000 0000 0000 0000 0001 */ + 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ + 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ + 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ + 0xffffffff /* 1111 1111 1111 1111 1111 1111 1111 1111 */ + }; + + /* " ", "#", "%", ";", "?", not allowed */ + + static uint32_t uri_path[] = { + 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ + + /* ?>=< ;:98 7654 3210 /.-, +*)( '&%$ #"! */ + 0xd800002d, /* 1101 1000 0000 0000 0000 0000 0010 1101 */ + + /* _^]\ [ZYX WVUT SRQP ONML KJIH GFED CBA@ */ + 0x50000000, /* 0101 0000 0000 0000 0000 0000 0000 0000 */ + + /* ~}| {zyx wvut srqp onml kjih gfed cba` */ + 0xb8000001, /* 1011 1000 0000 0000 0000 0000 0000 0001 */ + 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ @@ -1656,12 +1676,11 @@ ngx_escape_uri(u_char *dst, u_char *src, size_t size, ngx_uint_t type) static uint32_t *map[] = { uri, args, uri_component, html, refresh, memcached, memcached, - mail_xtext }; + mail_xtext, uri_path }; static u_char map_char[] = { '%', '%', '%', '%', '%', '%', '%', '+' }; - escape = map[type]; prefix = map_char[type]; diff --git a/src/core/ngx_string.h b/src/core/ngx_string.h index 183a20521..0c643ed69 100644 --- a/src/core/ngx_string.h +++ b/src/core/ngx_string.h @@ -204,6 +204,7 @@ u_char *ngx_utf8_cpystrn(u_char *dst, u_char *src, size_t n, size_t len); #define NGX_ESCAPE_MEMCACHED 5 #define NGX_ESCAPE_MAIL_AUTH 6 #define NGX_ESCAPE_MAIL_XTEXT 7 +#define NGX_ESCAPE_URI_PATH 8 #define NGX_UNESCAPE_URI 1 #define NGX_UNESCAPE_REDIRECT 2 diff --git a/src/http/modules/ngx_http_proxy_module.c b/src/http/modules/ngx_http_proxy_module.c index 7897b3f4b..b08a454c4 100644 --- a/src/http/modules/ngx_http_proxy_module.c +++ b/src/http/modules/ngx_http_proxy_module.c @@ -1133,12 +1133,8 @@ ngx_http_proxy_create_key(ngx_http_request_t *r) loc_len = (r->valid_location && ctx->vars.uri.len) ? ngx_min(plcf->location.len, r->uri.len) : 0; - 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); - } else { - escape = 0; - } + escape = 2 * ngx_escape_uri(NULL, r->uri.data + loc_len, + r->uri.len - loc_len, NGX_ESCAPE_URI_PATH); len = ctx->vars.uri.len + r->uri.len - loc_len + escape + sizeof("?") - 1 + r->args.len; @@ -1156,7 +1152,7 @@ ngx_http_proxy_create_key(ngx_http_request_t *r) if (escape) { ngx_escape_uri(p, r->uri.data + loc_len, - r->uri.len - loc_len, NGX_ESCAPE_URI); + r->uri.len - loc_len, NGX_ESCAPE_URI_PATH); p += r->uri.len - loc_len + escape; } else { @@ -1246,10 +1242,8 @@ ngx_http_proxy_create_request(ngx_http_request_t *r) loc_len = (r->valid_location && ctx->vars.uri.len) ? ngx_min(plcf->location.len, r->uri.len) : 0; - 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, r->uri.data + loc_len, + r->uri.len - loc_len, NGX_ESCAPE_URI_PATH); uri_len = ctx->vars.uri.len + r->uri.len - loc_len + escape + sizeof("?") - 1 + r->args.len; @@ -1373,7 +1367,7 @@ ngx_http_proxy_create_request(ngx_http_request_t *r) if (escape) { ngx_escape_uri(b->last, r->uri.data + loc_len, - r->uri.len - loc_len, NGX_ESCAPE_URI); + r->uri.len - loc_len, NGX_ESCAPE_URI_PATH); b->last += r->uri.len - loc_len + escape; } else { diff --git a/src/http/modules/ngx_http_proxy_v2_module.c b/src/http/modules/ngx_http_proxy_v2_module.c index 17983a29d..6d4bf1c8d 100644 --- a/src/http/modules/ngx_http_proxy_v2_module.c +++ b/src/http/modules/ngx_http_proxy_v2_module.c @@ -404,10 +404,8 @@ ngx_http_proxy_v2_create_request(ngx_http_request_t *r) loc_len = (r->valid_location && ctx->ctx.vars.uri.len) ? ngx_min(plcf->location.len, r->uri.len) : 0; - 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, r->uri.data + loc_len, + r->uri.len - loc_len, NGX_ESCAPE_URI_PATH); uri_len = ctx->ctx.vars.uri.len + r->uri.len - loc_len + escape + sizeof("?") - 1 + r->args.len; @@ -650,7 +648,7 @@ ngx_http_proxy_v2_create_request(ngx_http_request_t *r) if (escape) { ngx_escape_uri(p, r->uri.data + loc_len, - r->uri.len - loc_len, NGX_ESCAPE_URI); + r->uri.len - loc_len, NGX_ESCAPE_URI_PATH); p += r->uri.len - loc_len + escape; } else {