[PATCH] Fixed left-shifting of signed integers into the sign bit

Maxim Dounin mdounin at mdounin.ru
Wed Jul 23 19:05:11 UTC 2025


# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1753296799 -10800
#      Wed Jul 23 21:53:19 2025 +0300
# Node ID bdfd605f661eea3d272caf1bd5d85e7c539394ca
# Parent  e0792bb674b06cbb3c8a68ebff8d32b06a99cb97
Fixed left-shifting of signed integers into the sign bit.

If a bitwise left shift operation is used on a signed operand, and the
resulting value cannot be represented in the result type, the behaviour
is undefined.

In all cases in question the result is converted to the appropriate
unsigned type, and this doesn't cause any issues with known compilers.
Still, avoiding undefined behaviour is generally a good idea, only needs
a cast, and we already do this in multiple other places.  See also
6627:ad736705a744 and 6626:b3682580c1bd.

Prodded by UndefinedBehaviorSanitizer, "-fsanitize=shift".

diff --git a/src/core/ngx_inet.c b/src/core/ngx_inet.c
--- a/src/core/ngx_inet.c
+++ b/src/core/ngx_inet.c
@@ -507,7 +507,7 @@ ngx_cidr_match(struct sockaddr *sa, ngx_
 
             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/core/ngx_murmurhash.c b/src/core/ngx_murmurhash.c
--- a/src/core/ngx_murmurhash.c
+++ b/src/core/ngx_murmurhash.c
@@ -19,7 +19,7 @@ ngx_murmur_hash2(u_char *data, size_t le
         k  = data[0];
         k |= data[1] << 8;
         k |= data[2] << 16;
-        k |= data[3] << 24;
+        k |= (uint32_t) data[3] << 24;
 
         k *= 0x5bd1e995;
         k ^= k >> 24;
diff --git a/src/core/ngx_resolver.c b/src/core/ngx_resolver.c
--- a/src/core/ngx_resolver.c
+++ b/src/core/ngx_resolver.c
@@ -2194,7 +2194,7 @@ ngx_resolver_process_a(ngx_resolver_t *r
         type = (an->type_hi << 8) + an->type_lo;
         class = (an->class_hi << 8) + an->class_lo;
         len = (an->len_hi << 8) + an->len_lo;
-        ttl = (an->ttl[0] << 24) + (an->ttl[1] << 16)
+        ttl = ((uint32_t) an->ttl[0] << 24) + (an->ttl[1] << 16)
             + (an->ttl[2] << 8) + (an->ttl[3]);
 
         if (class != 1) {
@@ -2356,7 +2356,7 @@ ngx_resolver_process_a(ngx_resolver_t *r
 
             if (type == NGX_RESOLVE_A) {
 
-                addr[j] = htonl((buf[i] << 24) + (buf[i + 1] << 16)
+                addr[j] = htonl(((uint32_t) buf[i] << 24) + (buf[i + 1] << 16)
                                 + (buf[i + 2] << 8) + (buf[i + 3]));
 
                 if (++j == naddrs) {
@@ -2736,7 +2736,7 @@ ngx_resolver_process_srv(ngx_resolver_t 
         type = (an->type_hi << 8) + an->type_lo;
         class = (an->class_hi << 8) + an->class_lo;
         len = (an->len_hi << 8) + an->len_lo;
-        ttl = (an->ttl[0] << 24) + (an->ttl[1] << 16)
+        ttl = ((uint32_t) an->ttl[0] << 24) + (an->ttl[1] << 16)
             + (an->ttl[2] << 8) + (an->ttl[3]);
 
         if (class != 1) {
@@ -3142,7 +3142,7 @@ ngx_resolver_process_ptr(ngx_resolver_t 
             goto invalid_in_addr_arpa;
         }
 
-        addr += octet << mask;
+        addr += (in_addr_t) octet << mask;
         i += len;
     }
 
@@ -3301,7 +3301,7 @@ valid:
         type = (an->type_hi << 8) + an->type_lo;
         class = (an->class_hi << 8) + an->class_lo;
         len = (an->len_hi << 8) + an->len_lo;
-        ttl = (an->ttl[0] << 24) + (an->ttl[1] << 16)
+        ttl = ((uint32_t) an->ttl[0] << 24) + (an->ttl[1] << 16)
             + (an->ttl[2] << 8) + (an->ttl[3]);
 
         if (class != 1) {
diff --git a/src/http/modules/ngx_http_access_module.c b/src/http/modules/ngx_http_access_module.c
--- 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
         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
--- 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_requ
         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_req
             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
--- 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 *
         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
--- 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_ses
         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
--- 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_
         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
             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
--- 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
         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];



More information about the nginx-devel mailing list