From ead9ab09255042559c5568cb5959a487fbef2fab Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Wed, 21 Apr 2021 23:24:48 +0300 Subject: Version bump. --- src/core/nginx.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/core') diff --git a/src/core/nginx.h b/src/core/nginx.h index 8a3560179..d8c8289a8 100644 --- a/src/core/nginx.h +++ b/src/core/nginx.h @@ -9,8 +9,8 @@ #define _NGINX_H_INCLUDED_ -#define nginx_version 1019010 -#define NGINX_VERSION "1.19.10" +#define nginx_version 1021000 +#define NGINX_VERSION "1.21.0" #define NGINX_VER "nginx/" NGINX_VERSION #ifdef NGX_BUILD -- cgit From 6029e211c63895c44a942bacc32c6d2f565cc3e3 Mon Sep 17 00:00:00 2001 From: Ruslan Ermilov Date: Wed, 19 May 2021 16:24:13 +0300 Subject: Core: fixed comment about msie_refresh escaping. After 12a656452ad1, the "%" character is no longer escaped by ngx_escape_uri(NGX_ESCAPE_REFRESH). --- src/core/ngx_string.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/core') diff --git a/src/core/ngx_string.c b/src/core/ngx_string.c index 93f32ea0c..5cc9b26f9 100644 --- a/src/core/ngx_string.c +++ b/src/core/ngx_string.c @@ -1573,7 +1573,7 @@ ngx_escape_uri(u_char *dst, u_char *src, size_t size, ngx_uint_t type) 0xffffffff /* 1111 1111 1111 1111 1111 1111 1111 1111 */ }; - /* " ", """, "%", "'", %00-%1F, %7F-%FF */ + /* " ", """, "'", %00-%1F, %7F-%FF */ static uint32_t refresh[] = { 0xffffffff, /* 1111 1111 1111 1111 1111 1111 1111 1111 */ -- cgit From 9f1dcb0c0473641730b871dee984016ff19d2c53 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Tue, 25 May 2021 15:17:36 +0300 Subject: Resolver: fixed off-by-one write in ngx_resolver_copy(). Reported by Luis Merino, Markus Vervier, Eric Sesterhenn, X41 D-Sec GmbH. --- src/core/ngx_resolver.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/core') diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c index 793907010..63b26193d 100644 --- a/src/core/ngx_resolver.c +++ b/src/core/ngx_resolver.c @@ -4008,15 +4008,15 @@ done: n = *src++; } else { + if (dst != name->data) { + *dst++ = '.'; + } + ngx_strlow(dst, src, n); dst += n; src += n; n = *src++; - - if (n != 0) { - *dst++ = '.'; - } } if (n == 0) { -- cgit From 077a890a76fff4f071776184aed881b5f314c98a Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Tue, 25 May 2021 15:17:38 +0300 Subject: Resolver: fixed off-by-one read in ngx_resolver_copy(). It is believed to be harmless, and in the worst case it uses some uninitialized memory as a part of the compression pointer length, eventually leading to the "name is out of DNS response" error. --- src/core/ngx_resolver.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src/core') diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c index 63b26193d..9b1317234 100644 --- a/src/core/ngx_resolver.c +++ b/src/core/ngx_resolver.c @@ -3958,6 +3958,11 @@ ngx_resolver_copy(ngx_resolver_t *r, ngx_str_t *name, u_char *buf, u_char *src, } if (n & 0xc0) { + if (p >= last) { + err = "name is out of DNS response"; + goto invalid; + } + n = ((n & 0x3f) << 8) + *p; p = &buf[n]; -- cgit From bbd403a7ab3810fe82ecd1d6ebca9fc34d68126a Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Tue, 25 May 2021 15:17:41 +0300 Subject: Resolver: fixed label types handling in ngx_resolver_copy(). Previously, anything with any of the two high bits set were interpreted as compression pointers. This is incorrect, as RFC 1035 clearly states that "The 10 and 01 combinations are reserved for future use". Further, the 01 combination is actually allocated for EDNS extended label type (see RFC 2671 and RFC 6891), not really used though. Fix is to reject unrecognized label types rather than misinterpreting them as compression pointers. --- src/core/ngx_resolver.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src/core') diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c index 9b1317234..12dab09ea 100644 --- a/src/core/ngx_resolver.c +++ b/src/core/ngx_resolver.c @@ -3958,6 +3958,11 @@ ngx_resolver_copy(ngx_resolver_t *r, ngx_str_t *name, u_char *buf, u_char *src, } if (n & 0xc0) { + if ((n & 0xc0) != 0xc0) { + err = "invalid label type in DNS response"; + goto invalid; + } + if (p >= last) { err = "name is out of DNS response"; goto invalid; -- cgit From f1dd1d50e090b32a765295daea5f167f1077d706 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Tue, 25 May 2021 15:17:43 +0300 Subject: Resolver: reworked ngx_resolver_copy() copy loop. To make the code easier to read, reworked the ngx_resolver_copy() copy loop to match the one used to calculate length. No functional changes. --- src/core/ngx_resolver.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) (limited to 'src/core') diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c index 12dab09ea..0d7fd7915 100644 --- a/src/core/ngx_resolver.c +++ b/src/core/ngx_resolver.c @@ -4008,15 +4008,18 @@ done: name->data = dst; - n = *src++; - for ( ;; ) { + n = *src++; + + if (n == 0) { + name->len = dst - name->data; + return NGX_OK; + } + if (n & 0xc0) { n = ((n & 0x3f) << 8) + *src; src = &buf[n]; - n = *src++; - } else { if (dst != name->data) { *dst++ = '.'; @@ -4025,13 +4028,6 @@ done: ngx_strlow(dst, src, n); dst += n; src += n; - - n = *src++; - } - - if (n == 0) { - name->len = dst - name->data; - return NGX_OK; } } } -- cgit From f85d7016949b34119b5f4c53ddbfac4f199b4343 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Tue, 25 May 2021 15:17:45 +0300 Subject: Resolver: simplified ngx_resolver_copy(). Instead of checking on each label if we need to place a dot or not, now it always adds a dot after a label, and reduces the resulting length afterwards. --- src/core/ngx_resolver.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) (limited to 'src/core') diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c index 0d7fd7915..9ce53b930 100644 --- a/src/core/ngx_resolver.c +++ b/src/core/ngx_resolver.c @@ -3939,11 +3939,11 @@ ngx_resolver_copy(ngx_resolver_t *r, ngx_str_t *name, u_char *buf, u_char *src, { char *err; u_char *p, *dst; - ssize_t len; + size_t len; ngx_uint_t i, n; p = src; - len = -1; + len = 0; /* * compression pointers allow to create endless loop, so we set limit; @@ -3996,7 +3996,7 @@ done: return NGX_OK; } - if (len == -1) { + if (len == 0) { ngx_str_null(name); return NGX_OK; } @@ -4012,7 +4012,7 @@ done: n = *src++; if (n == 0) { - name->len = dst - name->data; + name->len = dst - name->data - 1; return NGX_OK; } @@ -4021,13 +4021,10 @@ done: src = &buf[n]; } else { - if (dst != name->data) { - *dst++ = '.'; - } - ngx_strlow(dst, src, n); dst += n; src += n; + *dst++ = '.'; } } } -- cgit From e860ecce82f1ee9cffb228d29d3ad61375b29aff Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Tue, 25 May 2021 15:17:50 +0300 Subject: Resolver: explicit check for compression pointers in question. Since nginx always uses exactly one entry in the question section of a DNS query, and never uses compression pointers in this entry, parsing of a DNS response in ngx_resolver_process_response() does not expect compression pointers to appear in the question section of the DNS response. Indeed, compression pointers in the first name of a DNS response hardly make sense, do not seem to be allowed by RFC 1035 (which says "a pointer to a prior occurance of the same name", note "prior"), and were never observed in practice. Added an explicit check to ngx_resolver_process_response()'s parsing of the question section to properly report an error if compression pointers nevertheless appear in the question section. --- src/core/ngx_resolver.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src/core') diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c index 9ce53b930..58d5f3ec4 100644 --- a/src/core/ngx_resolver.c +++ b/src/core/ngx_resolver.c @@ -1798,6 +1798,12 @@ ngx_resolver_process_response(ngx_resolver_t *r, u_char *buf, size_t n, i = sizeof(ngx_resolver_hdr_t); while (i < (ngx_uint_t) n) { + + if (buf[i] & 0xc0) { + err = "unexpected compression pointer in DNS response"; + goto done; + } + if (buf[i] == '\0') { goto found; } -- cgit