linux-hardening - RE: [PATCH 1/1] lib/vsprintf: Implement ssprintf() to catch truncated strings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 29 Jan 2024 09:39:28 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Lee Jones' <lee@...nel.org>
CC: Rasmus Villemoes <linux@...musvillemoes.dk>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org>, "Andrew
 Morton" <akpm@...ux-foundation.org>, Petr Mladek <pmladek@...e.com>, "Steven
 Rostedt" <rostedt@...dmis.org>, Andy Shevchenko
	<andriy.shevchenko@...ux.intel.com>, Sergey Senozhatsky
	<senozhatsky@...omium.org>, Crutcher Dunnavant
	<crutcher+kernel@...astacks.com>, Juergen Quade <quade@...r.de>
Subject: RE: [PATCH 1/1] lib/vsprintf: Implement ssprintf() to catch truncated
 strings

...
> > I'm sure that the safest return for 'truncated' is the buffer length.
> > The a series of statements like:
> > 	buf += xxx(buf, buf_end - buf, .....);
> > can all be called with a single overflow check at the end.
> >
> > Forget the check, and the length just contains a trailing '\0'
> > which might cause confusion but isn't going to immediately
> > break the world.
> 
> snprintf() does this and has been proven to cause buffer-overflows.
> There have been multiple articles authored describing why using
> snprintf() is not generally a good idea for the masses including the 2
> linked in the commit message:

snprintf() returns the number of bytes that would have been output [1].
I'm not suggesting that, or not terminating the buffer.
Just returning the length including the '\0' (unless length was zero).
This still lets the code check for overflow but isn't going to
generate a pointer outside the buffer if used to update a pointer.

[1] I'm pretty certain this is because the original libc version
of sprintf() allocated a FILE structure on stack (fully buffered)
and called fprintf().
snprintf() would have been done the same way but with something
to stop the buffer being flushed.

	David

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