[musl] [PATCH] src: network: replace unsafe strcat with strncat in getnameinfo.c

[musl] [PATCH] src: network: replace unsafe strcat with strncat in getnameinfo.c

[Anton Moryakov]

Replaced the vulnerable `strcat` function with `strncat` to prevent
potential buffer overflow. The new implementation limits the number
of characters copied to the remaining space in the destination buffer,
ensuring safe string concatenation.

The change addresses the following warning:
/.build/src/network/getnameinfo.c:178
Use of vulnerable function 'strcat' at getnameinfo.c:178. This function
is unsafe, use strncat instead.

While it is unclear if the static analyzer correctly identified this
as a vulnerability, it is better to err on the side of caution and
make the code safer by using `strncat`. The fix calculates the
available space in the buffer using `sizeof(buf) - strlen(buf) - 1`
to leave room for the null terminator.

Triggers found by static analyzer Svace.

Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>

---
 src/network/getnameinfo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/network/getnameinfo.c b/src/network/getnameinfo.c
index 133c15b3..e15f4457 100644
--- a/src/network/getnameinfo.c
+++ b/src/network/getnameinfo.c
@@ -179,7 +179,7 @@ int getnameinfo(const struct sockaddr *restrict sa, socklen_t sl,
 				if (!p)
 					p = itoa(num, scopeid);
 				*--p = '%';
-				strcat(buf, p);
+				strncat(buf, p, sizeof(buf) - strlen(buf) - 1);
 			}
 		}
 		if (strlen(buf) >= nodelen) return EAI_OVERFLOW;
-- 
2.30.2

Re: [musl] [PATCH] src: network: replace unsafe strcat with strncat in getnameinfo.c

[Rich Felker]

On Tue, Feb 25, 2025 at 04:26:46PM +0300, Anton Moryakov wrote:
> Replaced the vulnerable `strcat` function with `strncat` to prevent
> potential buffer overflow. The new implementation limits the number
> of characters copied to the remaining space in the destination buffer,
> ensuring safe string concatenation.
> 
> The change addresses the following warning:
> /.build/src/network/getnameinfo.c:178
> Use of vulnerable function 'strcat' at getnameinfo.c:178. This function
> is unsafe, use strncat instead.
> 
> While it is unclear if the static analyzer correctly identified this
> as a vulnerability, it is better to err on the side of caution and
> make the code safer by using `strncat`. The fix calculates the
> available space in the buffer using `sizeof(buf) - strlen(buf) - 1`
> to leave room for the null terminator.
> 
> Triggers found by static analyzer Svace.
> 
> Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
> 
> ---
>  src/network/getnameinfo.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/network/getnameinfo.c b/src/network/getnameinfo.c
> index 133c15b3..e15f4457 100644
> --- a/src/network/getnameinfo.c
> +++ b/src/network/getnameinfo.c
> @@ -179,7 +179,7 @@ int getnameinfo(const struct sockaddr *restrict sa, socklen_t sl,
>  				if (!p)
>  					p = itoa(num, scopeid);
>  				*--p = '%';
> -				strcat(buf, p);
> +				strncat(buf, p, sizeof(buf) - strlen(buf) - 1);
>  			}
>  		}
>  		if (strlen(buf) >= nodelen) return EAI_OVERFLOW;
> -- 
> 2.30.2

buf is 256 bytes. inet_ntop returns at most INET6_ADDRSTRLEN (46)
bytes. p points to a string of length at most IF_NAMESIZE+1 (17) or
3*sizeof(int) (12) bytes.

Rich