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.

diff --git a/src/core/ngx_inet.c b/src/core/ngx_inet.c
index 4228504adebb5d8f6a7ec59e684d1e3ad5bf9e93..acb2ef48a1e943f2d26d1eca438a67ab44e0c436 100644 (file)
--- 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 7355de9e735acda2a28c62158d3409bc0babb1f3..ea755200d0bae10a2d4be2ddd341e636e5ba6f12 100644 (file)
--- 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 8496b651af88f0e92932a246ade3789205c7ed73..75c03978a4ceb40af54ea904b2a9f1798564a26a 100644 (file)
--- 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 eaf98764f8713b72baa6707f89be9c79eb83a223..5b8b11fc79cf68d5196322e47db5aa7b2fe0142e 100644 (file)
--- 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 a3020d4fbd68acbe22bec38dfa51bd1892a12253..070c22614f52c2f4c53e48d9ca6af30b48e09c20 100644 (file)
--- 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 a9e10100f7b8e129d66031f6b120026b22bc7120..2324bef0df666a4a2c7e52609ac13f85b754d3fa 100644 (file)
--- 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 6507b716ec1a3ee2f00a93a2e290a44d0edeb989..3ee8f0e33e69c3a4b1f10b6c1c201c0ea869f60b 100644 (file)
--- 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];