[PATCH 0/3] daemon: fix network address handling bugs

[PATCH 0/3] daemon: fix network address handling bugs

[Sebastien Tardif via GitGitGadget]

Fix three related issues in daemon.c's network address handling:

IPv6 address corruption in lookup_hostname(): getaddrinfo() is called with
AF_UNSPEC hints, so it may return IPv6 results. However, the code
unconditionally casts ai_addr to sockaddr_in and passes AF_INET to
inet_ntop(). On IPv6-only hosts, this reads from the wrong struct offset,
producing garbage IP addresses. Fixed by checking ai_family and handling
both AF_INET and AF_INET6.

IPv6 address truncation in ip2str(): The sockaddr struct size (ai_addrlen)
is passed as the output buffer size to inet_ntop(). For IPv6,
sizeof(sockaddr_in6) is 28 bytes but INET6_ADDRSTRLEN is 46, so long IPv6
addresses are silently truncated. Fixed by passing sizeof(ip) instead, and
dropping the now-unused len parameter.

NULL pointer in execute() logging: REMOTE_PORT environment variable is used
in a format string without a NULL check (only REMOTE_ADDR was checked). If
REMOTE_PORT is unset, NULL is passed to printf's %s, which is undefined
behavior. Fixed by using a fallback string.

Sebastien Tardif (3):
  daemon: fix IPv6 address corruption in lookup_hostname()
  daemon: fix IPv6 address truncation in ip2str()
  daemon: guard NULL REMOTE_PORT in execute() logging

 daemon.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)


base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2300%2FSebTardif%2Ffix%2Fdaemon-ipv6-and-null-port-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2300/SebTardif/fix/daemon-ipv6-and-null-port-v1
Pull-Request: https://github.com/git/git/pull/2300
-- 
gitgitgadget

[PATCH 1/3] daemon: fix IPv6 address corruption in lookup_hostname()

[Sebastien Tardif via GitGitGadget]

From: Sebastien Tardif <sebtardif@ncf.ca>

getaddrinfo() is called with AF_UNSPEC hints, so it may return IPv6
results. However, the code unconditionally casts ai_addr to
sockaddr_in and passes AF_INET to inet_ntop(). On IPv6-only hosts,
this reads from the wrong struct offset, producing garbage IP
addresses.

Fix this by checking ai_family and extracting the address pointer
into a local variable before calling inet_ntop() once with the
correct family. Die on unexpected address families.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
---
 daemon.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/daemon.c b/daemon.c
index 0a7b1aae44..80fa0226d8 100644
--- a/daemon.c
+++ b/daemon.c
@@ -674,9 +674,20 @@ static void lookup_hostname(struct hostinfo *hi)
 
 		gai = getaddrinfo(hi->hostname.buf, NULL, &hints, &ai);
 		if (!gai) {
-			struct sockaddr_in *sin_addr = (void *)ai->ai_addr;
+			void *addr;
+
+			if (ai->ai_family == AF_INET) {
+				struct sockaddr_in *sa = (void *)ai->ai_addr;
+				addr = &sa->sin_addr;
+			} else if (ai->ai_family == AF_INET6) {
+				struct sockaddr_in6 *sa6 = (void *)ai->ai_addr;
+				addr = &sa6->sin6_addr;
+			} else {
+				die("unexpected address family: %d",
+				    ai->ai_family);
+			}
 
-			inet_ntop(AF_INET, &sin_addr->sin_addr,
+			inet_ntop(ai->ai_family, addr,
 				  addrbuf, sizeof(addrbuf));
 			strbuf_addstr(&hi->ip_address, addrbuf);
 
-- 
gitgitgadget

[PATCH 2/3] daemon: fix IPv6 address truncation in ip2str()

[Sebastien Tardif via GitGitGadget]

From: Sebastien Tardif <sebtardif@ncf.ca>

The sockaddr struct size (ai_addrlen) is passed as the output buffer
size to inet_ntop(). For IPv6, sizeof(sockaddr_in6) is 28 bytes but
INET6_ADDRSTRLEN is 46, so long IPv6 addresses are silently truncated.

Fix this by passing sizeof(ip) instead, which is the actual size of
the destination buffer. Drop the now-unused len parameter from
ip2str() and update all callers.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
---
 daemon.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/daemon.c b/daemon.c
index 80fa0226d8..103c08d868 100644
--- a/daemon.c
+++ b/daemon.c
@@ -947,7 +947,7 @@ struct socketlist {
 	size_t alloc;
 };
 
-static const char *ip2str(int family, struct sockaddr *sin, socklen_t len)
+static const char *ip2str(int family, struct sockaddr *sin)
 {
 #ifdef NO_IPV6
 	static char ip[INET_ADDRSTRLEN];
@@ -958,11 +958,11 @@ static const char *ip2str(int family, struct sockaddr *sin, socklen_t len)
 	switch (family) {
 #ifndef NO_IPV6
 	case AF_INET6:
-		inet_ntop(family, &((struct sockaddr_in6*)sin)->sin6_addr, ip, len);
+		inet_ntop(family, &((struct sockaddr_in6*)sin)->sin6_addr, ip, sizeof(ip));
 		break;
 #endif
 	case AF_INET:
-		inet_ntop(family, &((struct sockaddr_in*)sin)->sin_addr, ip, len);
+		inet_ntop(family, &((struct sockaddr_in*)sin)->sin_addr, ip, sizeof(ip));
 		break;
 	default:
 		xsnprintf(ip, sizeof(ip), "<unknown>");
@@ -1019,14 +1019,14 @@ static int setup_named_sock(char *listen_addr, int listen_port, struct socketlis
 
 		if (bind(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {
 			logerror("Could not bind to %s: %s",
-				 ip2str(ai->ai_family, ai->ai_addr, ai->ai_addrlen),
+				 ip2str(ai->ai_family, ai->ai_addr),
 				 strerror(errno));
 			close(sockfd);
 			continue;	/* not fatal */
 		}
 		if (listen(sockfd, 5) < 0) {
 			logerror("Could not listen to %s: %s",
-				 ip2str(ai->ai_family, ai->ai_addr, ai->ai_addrlen),
+				 ip2str(ai->ai_family, ai->ai_addr),
 				 strerror(errno));
 			close(sockfd);
 			continue;	/* not fatal */
@@ -1080,7 +1080,7 @@ static int setup_named_sock(char *listen_addr, int listen_port, struct socketlis
 
 	if ( bind(sockfd, (struct sockaddr *)&sin, sizeof sin) < 0 ) {
 		logerror("Could not bind to %s: %s",
-			 ip2str(AF_INET, (struct sockaddr *)&sin, sizeof(sin)),
+			 ip2str(AF_INET, (struct sockaddr *)&sin),
 			 strerror(errno));
 		close(sockfd);
 		return 0;
@@ -1088,7 +1088,7 @@ static int setup_named_sock(char *listen_addr, int listen_port, struct socketlis
 
 	if (listen(sockfd, 5) < 0) {
 		logerror("Could not listen to %s: %s",
-			 ip2str(AF_INET, (struct sockaddr *)&sin, sizeof(sin)),
+			 ip2str(AF_INET, (struct sockaddr *)&sin),
 			 strerror(errno));
 		close(sockfd);
 		return 0;
-- 
gitgitgadget

[PATCH 3/3] daemon: guard NULL REMOTE_PORT in execute() logging

[Sebastien Tardif via GitGitGadget]

From: Sebastien Tardif <sebtardif@ncf.ca>

The REMOTE_PORT environment variable is used in a format string
without a NULL check, while REMOTE_ADDR is checked. If REMOTE_PORT
is unset, NULL is passed to printf's %s, which is undefined behavior.

Add a fallback string for the NULL case.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
---
 daemon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/daemon.c b/daemon.c
index 103c08d868..78cca8673f 100644
--- a/daemon.c
+++ b/daemon.c
@@ -753,7 +753,7 @@ static int execute(void)
 	struct strvec env = STRVEC_INIT;
 
 	if (addr)
-		loginfo("Connection from %s:%s", addr, port);
+		loginfo("Connection from %s:%s", addr, port ? port : "?");
 
 	set_keep_alive(0);
 	alarm(init_timeout ? init_timeout : timeout);
-- 
gitgitgadget

Re: [PATCH 0/3] daemon: fix network address handling bugs

[Junio C Hamano]

"Sebastien Tardif via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Fix three related issues in daemon.c's network address handling:

Thanks for separating patches so that each of them addresses one
specific issue.

It would have been better if you sent this series as [PATCH v2] as a
reply to <pull.2299.git.git.1778291290159.gitgitgadget@gmail.com>,
which is the previous round.  That way, the mailing list archive
will keep the related discussions together on the same page.  If we
visit the page for the cover letter I am responding to,

  https://lore.kernel.org/git/pull.2300.git.git.1778773592.gitgitgadget@gmail.com/

nobody can see that there was a previous iteration so those who
looked at the earlier effort cannot refer back to it and compare.

> IPv6 address corruption in lookup_hostname(): getaddrinfo() is called with
> AF_UNSPEC hints, so it may return IPv6 results. However, the code
> unconditionally casts ai_addr to sockaddr_in and passes AF_INET to
> inet_ntop(). On IPv6-only hosts, this reads from the wrong struct offset,
> producing garbage IP addresses. Fixed by checking ai_family and handling
> both AF_INET and AF_INET6.
>
> IPv6 address truncation in ip2str(): The sockaddr struct size (ai_addrlen)
> is passed as the output buffer size to inet_ntop(). For IPv6,
> sizeof(sockaddr_in6) is 28 bytes but INET6_ADDRSTRLEN is 46, so long IPv6
> addresses are silently truncated. Fixed by passing sizeof(ip) instead, and
> dropping the now-unused len parameter.
>
> NULL pointer in execute() logging: REMOTE_PORT environment variable is used
> in a format string without a NULL check (only REMOTE_ADDR was checked). If
> REMOTE_PORT is unset, NULL is passed to printf's %s, which is undefined
> behavior. Fixed by using a fallback string.
>
> Sebastien Tardif (3):
>   daemon: fix IPv6 address corruption in lookup_hostname()
>   daemon: fix IPv6 address truncation in ip2str()
>   daemon: guard NULL REMOTE_PORT in execute() logging
>
>  daemon.c | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
>
>
> base-commit: 59ff4886a579f4bc91e976fe18590b9ae02c7a08
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2300%2FSebTardif%2Ffix%2Fdaemon-ipv6-and-null-port-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2300/SebTardif/fix/daemon-ipv6-and-null-port-v1
> Pull-Request: https://github.com/git/git/pull/2300

Re: [PATCH 1/3] daemon: fix IPv6 address corruption in lookup_hostname()

[Junio C Hamano]

"Sebastien Tardif via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Sebastien Tardif <sebtardif@ncf.ca>
>
> getaddrinfo() is called with AF_UNSPEC hints, so it may return IPv6
> results. However, the code unconditionally casts ai_addr to
> sockaddr_in and passes AF_INET to inet_ntop(). On IPv6-only hosts,
> this reads from the wrong struct offset, producing garbage IP
> addresses.
>
> Fix this by checking ai_family and extracting the address pointer
> into a local variable before calling inet_ntop() once with the
> correct family. Die on unexpected address families.
>
> Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
> ---
>  daemon.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/daemon.c b/daemon.c
> index 0a7b1aae44..80fa0226d8 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -674,9 +674,20 @@ static void lookup_hostname(struct hostinfo *hi)
>  
>  		gai = getaddrinfo(hi->hostname.buf, NULL, &hints, &ai);
>  		if (!gai) {
> -			struct sockaddr_in *sin_addr = (void *)ai->ai_addr;
> +			void *addr;
> +
> +			if (ai->ai_family == AF_INET) {
> +				struct sockaddr_in *sa = (void *)ai->ai_addr;
> +				addr = &sa->sin_addr;
> +			} else if (ai->ai_family == AF_INET6) {
> +				struct sockaddr_in6 *sa6 = (void *)ai->ai_addr;
> +				addr = &sa6->sin6_addr;
> +			} else {
> +				die("unexpected address family: %d",
> +				    ai->ai_family);
> +			}

The previous iteration used to more explicitly cast ai->ai_addr to
the target type, but the use of (void *) here is a cute way to make
the result shorter, which makes it a bit easier to read (it may take
readers a bit of practice to convince themselves that this type
conversion using (void *) as an intermediate type is perfectly fine,
though).

>  
> -			inet_ntop(AF_INET, &sin_addr->sin_addr,
> +			inet_ntop(ai->ai_family, addr,
>  				  addrbuf, sizeof(addrbuf));
>  			strbuf_addstr(&hi->ip_address, addrbuf);

Re: [PATCH 0/3] daemon: fix network address handling bugs

[Patrick Steinhardt]

On Fri, May 15, 2026 at 04:20:41AM +0900, Junio C Hamano wrote:
> "Sebastien Tardif via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > Fix three related issues in daemon.c's network address handling:
> 
> Thanks for separating patches so that each of them addresses one
> specific issue.
> 
> It would have been better if you sent this series as [PATCH v2] as a
> reply to <pull.2299.git.git.1778291290159.gitgitgadget@gmail.com>,
> which is the previous round.  That way, the mailing list archive
> will keep the related discussions together on the same page.  If we
> visit the page for the cover letter I am responding to,
> 
>   https://lore.kernel.org/git/pull.2300.git.git.1778773592.gitgitgadget@gmail.com/
> 
> nobody can see that there was a previous iteration so those who
> looked at the earlier effort cannot refer back to it and compare.

True. Other than that though I'm happy with this iteration. Thanks!

Patrick