|SUCCESS| pw159217 [PATCH v5 06/17] telemetry: avoid possible string overflow

[0-day Robot]

Test-Label: 0day Robot - Claude SONNET
Test-Status: SUCCESS
http://dpdk.org/patch/159217

_AI Review Comments Available_

Git SHA: efb48c9ed5
Author: Stephen Hemminger <stephen at networkplumber.org>
Subject: telemetry: avoid possible string overflow

This commit improves string handling in the legacy telemetry client
registration by avoiding temporary buffers and adding length checks for
unix domain socket paths.

> diff --git a/lib/telemetry/telemetry_legacy.c b/lib/telemetry/telemetry_legacy.c
> index 89ec750c09..24f663eee7 100644
> --- a/lib/telemetry/telemetry_legacy.c
> +++ b/lib/telemetry/telemetry_legacy.c
> @@ -82,34 +82,41 @@ static int
>  register_client(const char *cmd __rte_unused, const char *params,
>  		char *buffer __rte_unused, int buf_len __rte_unused)
>  {
> +	const char *colon = strchr(params, ':');
> +
> +	if (colon == NULL) {
> +		fprintf(stderr, "Invalid client data: missing colon\n");
> +		return -1;
> +	}
> +
>  #ifndef RTE_EXEC_ENV_WINDOWS
>  	pthread_t th;
> -	char data[BUF_SIZE];
> -	int fd;
> -	int rc;
> -	struct sockaddr_un addrs;
> -#endif /* !RTE_EXEC_ENV_WINDOWS */
> +	int fd, rc;
> +	struct sockaddr_un addrs = { .sun_family = AF_UNIX };
>  
> -	if (!strchr(params, ':')) {
> -		fprintf(stderr, "Invalid data\n");
> +	if (colon[1] != '"') {
> +		fprintf(stderr, "Invalid client data: missing quote\n");
>  		return -1;
>  	}
> -#ifndef RTE_EXEC_ENV_WINDOWS
> -	strlcpy(data, strchr(params, ':'), sizeof(data));
> -	memmove(data, &data[strlen(":\"")], strlen(data));
> -	if (!strchr(data, '\"')) {
> -		fprintf(stderr, "Invalid client data\n");
> +
> +	const char *endquote = strchr(colon + 1, '"');
> +	if (endquote == NULL) {
> +		fprintf(stderr, "Invalid client data: missing end quote\n");
> +		return -1;
> +	}
> +
> +	size_t len = endquote - colon + 1;
> +	if (len == 0 || len >= sizeof(addrs.sun_path)) {
> +		fprintf(stderr, "Invalid client data: path length\n");
>  		return -1;
>  	}
> -	*strchr(data, '\"') = 0;
> +	strncpy(addrs.sun_path, colon + 1, len);

Does this calculation for len include the opening quote in the path? The
calculation appears to be endquote - colon + 1, which would include the
quote character in the length, and then strncpy copies starting from
colon + 1 which still includes the quote character.

Would the string copy operation include the quote character in the socket
path string when it should be stripped out?

> +#endif /* !RTE_EXEC_ENV_WINDOWS */
>  
>  	fd = socket(AF_UNIX, SOCK_SEQPACKET, 0);
>  	if (fd < 0) {
>  		perror("Failed to open socket");
>  		return -1;
>  	}
> -	addrs.sun_family = AF_UNIX;
> -	strlcpy(addrs.sun_path, data, sizeof(addrs.sun_path));
>  
>  	if (connect(fd, (struct sockaddr *)&addrs, sizeof(addrs)) == -1) {
>  		perror("\nClient connection error\n");