Re: [PATCH][BZ #14771] Fortify tweak for snprintf et al.

[Rich Felker <dalias at aerifal dot cx>]

On Fri, Oct 18, 2013 at 01:30:07PM +0200, Florian Weimer wrote:
> +#ifndef _FORTIFY_H
> +#define _FORTIFY_H 1
> +
> +#include <limits.h>
> +
> +__BEGIN_DECLS
> +
> +/* Check that USER length does not exceed the COMPILER length (which
> +   should have been obtained through __builtin_object_size), and that
> +   USER does not exceed INT_MAX minus some reserve.
> +
> +   Technically, a caller to functions like snprintf could specify
> +   (size_t)-1 as the buffer length to get the behavior of sprintf
> +   (without target buffer length checking), but we do not support this
> +   use in fortify mode.  */
> +static inline void
> +__check_buffer_length_int_max (size_t user, size_t compiler)
> +{
> +  if (__builtin_expect (compiler < user || user >= INT_MAX - 1024, 0))
> +    __chk_fail ();
> +}

POSIX already requires an error from snprintf if the size passed to
snprintf is greater than INT_MAX, although this possibly conflicts
with the requirements of ISO C (this needs to be addressed by the ISO
C committee). If you believe there is any value in rejecting sizes
above INT_MAX-1024, I think you need a strong argument for doing so.
It's definitely contrary to the interface contract (which can thereby
introduce more bugs, including security bugs, than it eliminates) and
does not seem useful. Even on systems where size_t is 32-bit, negative
numbers will end up getting converted to values close to UINT_MAX,
nowhere near as low as INT_MAX. In what case would you expect values
just below INT_MAX to be the result of programming errors rather than
valid inputs?

Rich