From 3d5a356abb4f06b0f103290bd31a4c146233956b Mon Sep 17 00:00:00 2001 From: Sergey Kandaurov Date: Mon, 18 Mar 2024 17:14:30 +0400 Subject: [PATCH] Fixed undefined behaviour with IPv4-mapped IPv6 addresses. Previously, it could result when left-shifting signed integer due to implicit integer promotion, such that the most significant bit appeared on the sign bit. In practice, though, this results in the same left value as with an explicit cast, at least on known compilers, such as GCC and Clang. The reason is that in_addr_t, which is equivalent to uint32_t and same as "unsigned int" in ILP32 and LP64 data type models, has the same type width as the intermediate after integer promotion, so there's no side effects such as sign-extension. This explains why adding an explicit cast does not change object files in practice. Found with UndefinedBehaviorSanitizer (shift). Based on a patch by Piotr Sikora. --- src/core/ngx_inet.c | 2 +- src/http/modules/ngx_http_access_module.c | 2 +- src/http/modules/ngx_http_geo_module.c | 4 ++-- src/http/modules/ngx_http_geoip_module.c | 2 +- src/stream/ngx_stream_access_module.c | 2 +- src/stream/ngx_stream_geo_module.c | 4 ++-- src/stream/ngx_stream_geoip_module.c | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/core/ngx_inet.c b/src/core/ngx_inet.c index 4228504ad..acb2ef48a 100644 --- a/src/core/ngx_inet.c +++ b/src/core/ngx_inet.c @@ -507,7 +507,7 @@ ngx_cidr_match(struct sockaddr *sa, ngx_array_t *cidrs) p = inaddr6->s6_addr; - inaddr = p[12] << 24; + inaddr = (in_addr_t) p[12] << 24; inaddr += p[13] << 16; inaddr += p[14] << 8; inaddr += p[15]; diff --git a/src/http/modules/ngx_http_access_module.c b/src/http/modules/ngx_http_access_module.c index 7355de9e7..ea755200d 100644 --- a/src/http/modules/ngx_http_access_module.c +++ b/src/http/modules/ngx_http_access_module.c @@ -148,7 +148,7 @@ ngx_http_access_handler(ngx_http_request_t *r) p = sin6->sin6_addr.s6_addr; if (alcf->rules && IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr)) { - addr = p[12] << 24; + addr = (in_addr_t) p[12] << 24; addr += p[13] << 16; addr += p[14] << 8; addr += p[15]; diff --git a/src/http/modules/ngx_http_geo_module.c b/src/http/modules/ngx_http_geo_module.c index 8496b651a..75c03978a 100644 --- a/src/http/modules/ngx_http_geo_module.c +++ b/src/http/modules/ngx_http_geo_module.c @@ -199,7 +199,7 @@ ngx_http_geo_cidr_variable(ngx_http_request_t *r, ngx_http_variable_value_t *v, p = inaddr6->s6_addr; if (IN6_IS_ADDR_V4MAPPED(inaddr6)) { - inaddr = p[12] << 24; + inaddr = (in_addr_t) p[12] << 24; inaddr += p[13] << 16; inaddr += p[14] << 8; inaddr += p[15]; @@ -272,7 +272,7 @@ ngx_http_geo_range_variable(ngx_http_request_t *r, ngx_http_variable_value_t *v, if (IN6_IS_ADDR_V4MAPPED(inaddr6)) { p = inaddr6->s6_addr; - inaddr = p[12] << 24; + inaddr = (in_addr_t) p[12] << 24; inaddr += p[13] << 16; inaddr += p[14] << 8; inaddr += p[15]; diff --git a/src/http/modules/ngx_http_geoip_module.c b/src/http/modules/ngx_http_geoip_module.c index eaf98764f..5b8b11fc7 100644 --- a/src/http/modules/ngx_http_geoip_module.c +++ b/src/http/modules/ngx_http_geoip_module.c @@ -266,7 +266,7 @@ ngx_http_geoip_addr(ngx_http_request_t *r, ngx_http_geoip_conf_t *gcf) if (IN6_IS_ADDR_V4MAPPED(inaddr6)) { p = inaddr6->s6_addr; - inaddr = p[12] << 24; + inaddr = (in_addr_t) p[12] << 24; inaddr += p[13] << 16; inaddr += p[14] << 8; inaddr += p[15]; diff --git a/src/stream/ngx_stream_access_module.c b/src/stream/ngx_stream_access_module.c index a3020d4fb..070c22614 100644 --- a/src/stream/ngx_stream_access_module.c +++ b/src/stream/ngx_stream_access_module.c @@ -144,7 +144,7 @@ ngx_stream_access_handler(ngx_stream_session_t *s) p = sin6->sin6_addr.s6_addr; if (ascf->rules && IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr)) { - addr = p[12] << 24; + addr = (in_addr_t) p[12] << 24; addr += p[13] << 16; addr += p[14] << 8; addr += p[15]; diff --git a/src/stream/ngx_stream_geo_module.c b/src/stream/ngx_stream_geo_module.c index a9e10100f..2324bef0d 100644 --- a/src/stream/ngx_stream_geo_module.c +++ b/src/stream/ngx_stream_geo_module.c @@ -190,7 +190,7 @@ ngx_stream_geo_cidr_variable(ngx_stream_session_t *s, p = inaddr6->s6_addr; if (IN6_IS_ADDR_V4MAPPED(inaddr6)) { - inaddr = p[12] << 24; + inaddr = (in_addr_t) p[12] << 24; inaddr += p[13] << 16; inaddr += p[14] << 8; inaddr += p[15]; @@ -263,7 +263,7 @@ ngx_stream_geo_range_variable(ngx_stream_session_t *s, if (IN6_IS_ADDR_V4MAPPED(inaddr6)) { p = inaddr6->s6_addr; - inaddr = p[12] << 24; + inaddr = (in_addr_t) p[12] << 24; inaddr += p[13] << 16; inaddr += p[14] << 8; inaddr += p[15]; diff --git a/src/stream/ngx_stream_geoip_module.c b/src/stream/ngx_stream_geoip_module.c index 6507b716e..3ee8f0e33 100644 --- a/src/stream/ngx_stream_geoip_module.c +++ b/src/stream/ngx_stream_geoip_module.c @@ -236,7 +236,7 @@ ngx_stream_geoip_addr(ngx_stream_session_t *s, ngx_stream_geoip_conf_t *gcf) if (IN6_IS_ADDR_V4MAPPED(inaddr6)) { p = inaddr6->s6_addr; - inaddr = p[12] << 24; + inaddr = (in_addr_t) p[12] << 24; inaddr += p[13] << 16; inaddr += p[14] << 8; inaddr += p[15];