Sergey Kandaurov
March 27, 2024 01:48PM
details: https://hg.nginx.org/nginx/rev/f208413f307f
branches:
changeset: 9227:f208413f307f
user: Sergey Kandaurov <pluknet@nginx.com>
date: Mon Mar 18 17:14:30 2024 +0400
description:
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.

diffstat:

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(-)

diffs (102 lines):

diff -r 687c8306746f -r f208413f307f src/core/ngx_inet.c
--- a/src/core/ngx_inet.c Thu Mar 14 18:37:20 2024 +0400
+++ b/src/core/ngx_inet.c Mon Mar 18 17:14:30 2024 +0400
@@ -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 -r 687c8306746f -r f208413f307f src/http/modules/ngx_http_access_module.c
--- a/src/http/modules/ngx_http_access_module.c Thu Mar 14 18:37:20 2024 +0400
+++ b/src/http/modules/ngx_http_access_module.c Mon Mar 18 17:14:30 2024 +0400
@@ -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 -r 687c8306746f -r f208413f307f src/http/modules/ngx_http_geo_module.c
--- a/src/http/modules/ngx_http_geo_module.c Thu Mar 14 18:37:20 2024 +0400
+++ b/src/http/modules/ngx_http_geo_module.c Mon Mar 18 17:14:30 2024 +0400
@@ -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 -r 687c8306746f -r f208413f307f src/http/modules/ngx_http_geoip_module.c
--- a/src/http/modules/ngx_http_geoip_module.c Thu Mar 14 18:37:20 2024 +0400
+++ b/src/http/modules/ngx_http_geoip_module.c Mon Mar 18 17:14:30 2024 +0400
@@ -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 -r 687c8306746f -r f208413f307f src/stream/ngx_stream_access_module.c
--- a/src/stream/ngx_stream_access_module.c Thu Mar 14 18:37:20 2024 +0400
+++ b/src/stream/ngx_stream_access_module.c Mon Mar 18 17:14:30 2024 +0400
@@ -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 -r 687c8306746f -r f208413f307f src/stream/ngx_stream_geo_module.c
--- a/src/stream/ngx_stream_geo_module.c Thu Mar 14 18:37:20 2024 +0400
+++ b/src/stream/ngx_stream_geo_module.c Mon Mar 18 17:14:30 2024 +0400
@@ -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 -r 687c8306746f -r f208413f307f src/stream/ngx_stream_geoip_module.c
--- a/src/stream/ngx_stream_geoip_module.c Thu Mar 14 18:37:20 2024 +0400
+++ b/src/stream/ngx_stream_geoip_module.c Mon Mar 18 17:14:30 2024 +0400
@@ -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];
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[nginx] Fixed undefined behaviour with IPv4-mapped IPv6 addresses.

Sergey Kandaurov 246 March 27, 2024 01:48PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 178
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 500 on July 15, 2024
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready