oss-sec mailing list archives

Re: snprintf return value misuse in a lot of projects


From: Alexander Cherepanov <ch3root () openwall com>
Date: Sat, 13 Feb 2016 19:51:04 +0300

On 2016-02-13 17:11, Yuriy M. Kaminskiy wrote:
I noticed dangerous pattern in a lot of projects, where snprintf(3)
return value is used without checking, with potentially disasterous
consequences:

It's kinda a known. E.g., some such patterns are listed in https://sourceware.org/ml/libc-alpha/2013-10/msg00686.html .

The same problem is with strlcpy.

And there are yet another very common pattern:

   p += snprintf(p, end-p,[....]);
   p += snprintf(p, end-p,[....]);
   p += snprintf(p, end-p,[....]);
   ...

which may be 'barely safe' by posix (if you'd read `man 3posix snprintf`,
you'd expect 2nd line is [somewhat] safe (end-p is negative, then
casted to size_t and produce value larger than (size_t)INT_MAX, that
should result in error EOVERFLOW), and third and following will dance
around last byte, likely remaining safe), but it is TOTALLY
broken on glibc, as glibc's snprintf DOES NOT follow posix, and accepts
*any* size.

For a glibc discussion please see https://sourceware.org/bugzilla/show_bug.cgi?id=14771 .

As for POSIX, the requirement of EOVERFLOW for a big second parameter is a (rejected) bug in POSIX -- http://austingroupbugs.net/view.php?id=761 . A closely related bug -- http://austingroupbugs.net/view.php?id=1020 .

ISO C describes the size parameter of snprintf as a limit to the number of output characters written, without any connections to the size of the buffer. Thus, the following examples are valid in ISO C:

  char s[10];
  snprintf(s, 20, "abc");
  snprintf(s, SIZE_MAX, "%s", "abc");

OTOH POSIX describes the size parameter as the actual size of the buffer (bug 1020) and requires to reject buffers of size larger than INT_MAX (bug 761).

Even though POSIX contradicts ISO C in this question (while formally deferring to ISO C) there is a sentiment that the POSIX approach is better for safety/security. (E.g., it was expressed during the recent discussion about strlcpy/strlcat in the glibc mailing list.)

As it turned out, the same problem affects the fread function, with the Linux kernel instead of POSIX contradicting ISO C. See https://sourceware.org/bugzilla/show_bug.cgi?id=19165 and https://sourceware.org/ml/libc-alpha/2016-02/msg00274.html .

Perhaps this is a topic that will benefit from input from a wider community.

--
Alexander Cherepanov


Current thread: