[Announce] GnuPG 1.4 and 2.0 buffer overflow

Christian Biere christianbiere at gmx.de
Wed Nov 29 17:32:43 CET 2006


Werner Koch wrote:
> On Wed, 29 Nov 2006 11:52, christianbiere at gmx.de said:
> 
> > similar to asprintf() that takes no format string but simply a variable number of string arguments
> 
> Well in that concrete case that would be a good idea.  However the
> sentinel attribute is only available since gcc 4 and in the past it
> was common to miss the terminating NULL in the arg list ;-)

Sure, but assuming you're at least half-consistent about your coding style, a
very simple script (awk, Perl, whatever) could be used to check for this. Also
there are always typical mistakes the compiler won't check for. Calculating a
buffer size can be very tricky and is well-known to be error-prone. Forgetting
some terminator or typing "," instead of ";" and whatnot happens too but it's
not that tricky.

Also compilers with format string checks haven't been around forever either
and many code still is ignorant of this GCC feature. Even with these format
checks, bugs might be unnoticed for example if people pass a size_t, pid_t
or whatever implementation-defined type as %d (or %u). There might be
absolutely no warnings on the developer's machine. So the format string
solution has much more issues whereas the other one has mainly two:
forgetting to terminate the list with "(void *) 0" and accidently passing
a parameter that is NULL but that's easy to prevent and easy to check
statically.

> In general such a function will not help as it gets into the way when
> doing i18n.

Yes, unfortunately but not in this case and restricting printf-like printing to
those cases where it's needed or rather the straight-forward solution (i18n
implies pretty-printing) seems like a good practice.

Also replacement implementations for snprintf() have been around for years
and vsnprintf() can be used to write your own asprintf() in about 5 lines.
This should even work with the non-C99 variant, it just requires multiple
passes. If library functions are buggy, that's sad but really not your
problem. The library vendor should be screwed for that.
 
> > Also, asprintf() returns an int instead of size_t. Yet another library
> > function broken by design.
 
> This is perfectly reasonable.  How would you return an error code with
> size_t?

(size_t)-1 or ASPRINTF_ERROR?

-- 
Christian



More information about the Gnupg-devel mailing list