[PATCH v3 iproute2] ifstat: convert sprintf to snprintf

[PATCH v3 iproute2] ifstat: convert sprintf to snprintf

[Denis Kirjanov]

Use snprintf to print only valid data

v2: adjust formatting
v3: fix the issue with a buffer length

Signed-off-by: Denis Kirjanov <dkirjanov@suse.de>
---
 misc/ifstat.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/misc/ifstat.c b/misc/ifstat.c
index 721f4914..4ce550b2 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -379,10 +379,10 @@ static void format_rate(FILE *fp, const unsigned long long *vals,
 		fprintf(fp, "%8llu ", vals[i]);
 
 	if (rates[i] > mega) {
-		sprintf(temp, "%uM", (unsigned int)(rates[i]/mega));
+		snprintf(temp, sizeof(temp), "%uM", (unsigned int)(rates[i]/mega));
 		fprintf(fp, "%-6s ", temp);
 	} else if (rates[i] > kilo) {
-		sprintf(temp, "%uK", (unsigned int)(rates[i]/kilo));
+		snprintf(temp, sizeof(temp), "%uK", (unsigned int)(rates[i]/kilo));
 		fprintf(fp, "%-6s ", temp);
 	} else
 		fprintf(fp, "%-6u ", (unsigned int)rates[i]);
@@ -400,10 +400,10 @@ static void format_pair(FILE *fp, const unsigned long long *vals, int i, int k)
 		fprintf(fp, "%8llu ", vals[i]);
 
 	if (vals[k] > giga) {
-		sprintf(temp, "%uM", (unsigned int)(vals[k]/mega));
+		snprintf(temp, sizeof(temp), "%uM", (unsigned int)(vals[k]/mega));
 		fprintf(fp, "%-6s ", temp);
 	} else if (vals[k] > mega) {
-		sprintf(temp, "%uK", (unsigned int)(vals[k]/kilo));
+		snprintf(temp, sizeof(temp), "%uK", (unsigned int)(vals[k]/kilo));
 		fprintf(fp, "%-6s ", temp);
 	} else
 		fprintf(fp, "%-6u ", (unsigned int)vals[k]);
@@ -675,7 +675,7 @@ static void server_loop(int fd)
 	p.fd = fd;
 	p.events = p.revents = POLLIN;
 
-	sprintf(info_source, "%d.%lu sampling_interval=%d time_const=%d",
+	snprintf(info_source, sizeof(info_source), "%d.%lu sampling_interval=%d time_const=%d",
 		getpid(), (unsigned long)random(), scan_interval/1000, time_constant/1000);
 
 	load_info();
@@ -893,7 +893,7 @@ int main(int argc, char *argv[])
 
 	sun.sun_family = AF_UNIX;
 	sun.sun_path[0] = 0;
-	sprintf(sun.sun_path+1, "ifstat%d", getuid());
+	snprintf(sun.sun_path + 1, sizeof(sun.sun_path) - 1, "ifstat%d", getuid());
 
 	if (scan_interval > 0) {
 		if (time_constant == 0)
-- 
2.30.2

RE: [PATCH v3 iproute2] ifstat: convert sprintf to snprintf

[David Laight]

From: Denis Kirjanov
> Sent: 14 February 2024 12:57
> 
> Use snprintf to print only valid data

... to avoid potentially overflowed the temporary buffer.

Also probably worth using scnprintf() to avoid another change
when snprintf() is removed (because the return value is dangerous).

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH v3 iproute2] ifstat: convert sprintf to snprintf

[Stephen Hemminger]

On Sat, 17 Feb 2024 17:51:01 +0000
David Laight <David.Laight@ACULAB.COM> wrote:

> From: Denis Kirjanov
> > Sent: 14 February 2024 12:57
> > 
> > Use snprintf to print only valid data  
> 
> ... to avoid potentially overflowed the temporary buffer.
> 
> Also probably worth using scnprintf() to avoid another change
> when snprintf() is removed (because the return value is dangerous).
> 

No.
Read the thread, return value is almost never used in iproute2
and where it is, the checks are in place.

Re: [PATCH v3 iproute2] ifstat: convert sprintf to snprintf

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to iproute2/iproute2-next.git (main)
by David Ahern <dsahern@kernel.org>:

On Wed, 14 Feb 2024 07:56:59 -0500 you wrote:
> Use snprintf to print only valid data
> 
> v2: adjust formatting
> v3: fix the issue with a buffer length
> 
> Signed-off-by: Denis Kirjanov <dkirjanov@suse.de>
> 
> [...]

Here is the summary with links:
  - [v3,iproute2] ifstat: convert sprintf to snprintf
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=d2f1c3c9a8a3

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html