This is the mail archive of the
mailing list for the glibc project.
Re: ISO C11 issues for glibc
On Wed, 21 Dec 2011, Marek Polacek wrote:
> Which reminds me, what is your opinion on those *_s bounds-checking
> functions, e.g, vsnprintf_s etc, as specified in Annex K?
> I'd say we don't really need them since we have _FORTIFY_SOURCE
> and thus *_chk functions...
Apart from the reentrant functions (generally quite similar to existing
POSIX functions, but under different names), the functions are generally
designed for use in retrofitting large bodies of existing code to improve
its security without needing a detailed understanding of that code.
_FORTIFY_SOURCE is another way of retrofitting that requires no changes to
the code at all, but I think it can be useful to have both.
There is certainly a case that, for example, it's better coding style to
use snprintf rather than sprintf even when you *also* dynamically (or
statically) size your buffers so that snprintf should never cause
truncation. Silent truncation is bad, and could be a security hole, but
it's less likely to be a security hole than a buffer overrun, so if you
write your code to avoid arbitrary limits and silent truncation, a coding
style that always uses snprintf instead of sprintf can reduce the risk of
a bug in the code turning into a security hole by changing a buffer
overrun into truncation (if you passed the correct size; it can't solve
all problems, but can at least encourage you to think about sizes
Much the same applies to a few of the functions in Annex K, where the
corresponding existing functions do not have buffer sizes passed
explicitly; there is a use for coding styles that ensure that explicit
buffer sizes are always passed (and personally I would have allowed in
strlcpy/strlcat long ago for that reason; you don't want truncation ever
to happen, but they still help reduce the risk of some bugs, and glibc has
traditionally included functions that are widely used in libc on other
systems, which by now applies to those functions).
Other bits are rather stranger, such as tmpfile_s (adding a runtime check
that a pointer is not NULL) and tmpnam_s (which leaves a race condition
still present, so certainly shouldn't be used in new code; presumably it
was added on the basis that otherwise legacy code might not have tmpnam
calls retrofitted at all, and some fixing was better than none).
Then there's rsize_t and RSIZE_MAX, intended to limit size_t parameters to
less than SIZE_MAX (probably no more than SIZE_MAX >> 1). There is a real
issue here. With normal implementations of C, it is impossible for
objects half the address space or more in size to work reliably; pointer
subtraction within such an object will produce values that cannot be
represented in ptrdiff_t, so silently resulting in undefined behavior, and
in practice even if the pointer target type is larger than one byte, the
division happens after the subtraction and the results will be wrong (see
<http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45779>). And of course
pointer subtraction might be generated internally even if not in the
user's source code.
I don't think there's anything that can sensibly be done in the compiler
about this issue; I think the only way to avoid security problems there is
for malloc and other allocation functions to refuse to allocate objects
using half or more of the address space (so it would no longer be possible
to allocate 2GB or more in a single allocation on a 32-bit system; for
64-bit systems there would be little practical effect). (It's already the
case that GCC will give an error if you attempts to declare an object half
the address space or larger in (constant) size, whether statically
allocated or on the stack.) You don't need rsize_t for that, of course -
you can and should change the existing allocation functions to put in this
limit - and once you have such limits no objects can exceed RSIZE_MAX
anyway so the checks (in various Annex K functions) for passing sizes
bigger than RSIZE_MAX become just checks for some random possible bug in
the program, much like the checks for NULL.
Joseph S. Myers