Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [day] [month] [year] [list]
Date: Sun, 24 Mar 2024 13:46:29 -0400
From: Rich Felker <dalias@...ifal.cx>
To: Maks Mishin <maks.mishinfz@...il.com>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] sprintf: Replace call vsprintf to vsnprintf

On Sun, Mar 24, 2024 at 07:51:54PM +0300, Maks Mishin wrote:
> Use of vulnerable function 'vsprintf' at sprintf.c:9.
> This function is unsafe, use vsnprintf instead.
> 
> Found bu RASU JSC.
> 
> Signed-off-by: Maks Mishin <maks.mishinFZ@...il.com>
> ---
>  src/stdio/sprintf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/stdio/sprintf.c b/src/stdio/sprintf.c
> index 9dff524c..e6b2a411 100644
> --- a/src/stdio/sprintf.c
> +++ b/src/stdio/sprintf.c
> @@ -6,7 +6,7 @@ int sprintf(char *restrict s, const char *restrict fmt, ...)
>  	int ret;
>  	va_list ap;
>  	va_start(ap, fmt);
> -	ret = vsprintf(s, fmt, ap);
> +	ret = vsnprintf(s, sizeof s, fmt, ap);
>  	va_end(ap);
>  	return ret;
>  }
> -- 
> 2.30.2

This patch is clearly wrong. It passes the size of a pointer instead
of the size of a buffer.

Moreover, this is the actual implementation of sprintf, which does not
have access to the buffer size.

Please (1) send these mails to the list, not in private, and (2) don't
send patches that are just formulatic changes based on a static
analysis tool. *Every single one* so far has been wrong.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.