Jim Meyering wrote: > Nice and thorough work. Thanks for covering almost all of the > remaining cases.
OK, here's a proposed patch, for inttostr. If that works fine, I'll continue with the *sprintf variants in libunistring. 2010-10-17 Jim Meyering <meyer...@redhat.com> Bruno Haible <br...@clisp.org> Add bounds checking to the inttostr() and similar functions. * lib/anytostr.c (_GL_NO_FORTIFY): Define. Include <stdlib.h>. (anytostr_chk): New function. * lib/inttostr.h (_GL_STRINGIFY, _GL_CONCAT): New macros. (imaxtostr_chk, inttostr_chk, offtostr_chk, uinttostr_chk, umaxtostr_chk): New declarations. (_GL_pointed_object_size, _GL_ASM_SYMBOL_PREFIX): New macros. (_GL_DEFINE_INTTOSTR_FUNCS): New macro. (imaxtostr, inttostr, offtostr, uinttostr, umaxtostr): Redefine as macros with bounds checking. * lib/verify.h (_GL_CONCAT): Ensure no redefinition. * m4/inttostr.m4 (gl_PREREQ_INTTOSTR): Require gl_ASM_SYMBOL_PREFIX. * modules/inttostr (Files): Add m4/asm-underscore.m4. * modules/inttostr-tests (Files): Add tests/test-inttostr2.c. * tests/test-inttostr2.c: New file. --- lib/anytostr.c.orig Sun Oct 17 21:54:43 2010 +++ lib/anytostr.c Sun Oct 17 21:44:40 2010 @@ -19,7 +19,12 @@ #include <config.h> +/* Don't define inttostr etc. as macros in this compilation unit. */ +#define _GL_NO_FORTIFY 1 + #include "inttostr.h" + +#include <stdlib.h> #include "verify.h" /* Convert I to a printable string in BUF, which must be at least @@ -52,3 +57,16 @@ return p; } + +/* Like anytostr. + Also verify that SIZE is >= INT_BUFSIZE_BOUND (INTTYPE). + SIZE is the number of bytes available at BUF, as determined by the + compiler. */ +char * __attribute_warn_unused_result__ +_GL_CONCAT (anytostr, _chk) (inttype i, char *buf, size_t size) +{ + if (!(size >= INT_BUFSIZE_BOUND (inttype))) + /* The compiler determined that the buf argument is too small. */ + abort (); + return anytostr (i, buf); +} --- lib/inttostr.h.orig Sun Oct 17 21:54:43 2010 +++ lib/inttostr.h Sun Oct 17 21:54:39 2010 @@ -39,8 +39,111 @@ # define __attribute_warn_unused_result__ /* empty */ #endif +#ifndef _GL_STRINGIFY +/* Convert a token to a string. */ +# define _GL_STRINGIFY(s) _GL_STRINGIFY1 (s) +# define _GL_STRINGIFY1(s) #s +#endif + +#ifndef _GL_CONCAT +/* Concatenate two preprocessor tokens. */ +# define _GL_CONCAT(x, y) _GL_CONCAT0 (x, y) +# define _GL_CONCAT0(x, y) x##y +#endif + char *imaxtostr (intmax_t, char *) __attribute_warn_unused_result__; +char *imaxtostr_chk (intmax_t, char *, size_t) __attribute_warn_unused_result__; + char *inttostr (int, char *) __attribute_warn_unused_result__; +char *inttostr_chk (int, char *, size_t) __attribute_warn_unused_result__; + char *offtostr (off_t, char *) __attribute_warn_unused_result__; +char *offtostr_chk (off_t, char *, size_t) __attribute_warn_unused_result__; + char *uinttostr (unsigned int, char *) __attribute_warn_unused_result__; +char *uinttostr_chk (unsigned int, char *, size_t) __attribute_warn_unused_result__; + char *umaxtostr (uintmax_t, char *) __attribute_warn_unused_result__; +char *umaxtostr_chk (uintmax_t, char *, size_t) __attribute_warn_unused_result__; + +/* When, on glibc systems, -D_FORTIFY_SOURCE=1 or -D_FORTIFY_SOURCE=2 is used, + enable extra bounds checking, based on the object bounds analysis done by + GCC. + The user can disable this bounds checking by defining _GL_NO_FORTIFY. + __attribute__ __warning__ requires GCC >= 4.3. + __builtin_object_size requires GCC >= 4.1. + __always_inline__ requires GCC >= 3.2. */ +#if __USE_FORTIFY_LEVEL > 0 && !defined _GL_NO_FORTIFY && __GNUC_PREREQ (4, 3) + +/* Compiler determined size of a char* pointer. or char[] array. + Works via a GCC built-in for pointers into arrays of constant size, + and works via sizeof() for references to arrays of variable length. + Punt for pointers to arrays of variable length or unknown length. */ +# define _GL_pointed_object_size(s) \ + (__builtin_object_size (s, 1) != (size_t)-1 \ + ? __builtin_object_size (s, 1) \ + : sizeof (s) != sizeof (void *) \ + ? sizeof (s) \ + : (size_t)-1) + +/* The prefix of C symbols at the assembly language level and the linker level, + as a string. USER_LABEL_PREFIX is determined by gl_ASM_SYMBOL_PREFIX. */ +# define _GL_ASM_SYMBOL_PREFIX _GL_STRINGIFY (USER_LABEL_PREFIX) + +/* Define auxiliary functions for bounds checking of FUNC. + The FUNC_chk_warn function behaves like the FUNC_chk function but emits a + GCC compile-time warning attached to it. It has to be an 'extern' function, + not inline, due to the way GCC's __warning__ attribute works. + The FUNC_chk_inline function exists so that the arguments of FUNC are + evaluated only once, that is, so that side effects in the arguments happen + exactly once. */ +# define _GL_DEFINE_INTTOSTR_FUNCS(FUNC,TYPE) \ + extern char * \ + __attribute__(( \ + __warning__ (_GL_STRINGIFY(FUNC) ": size of destination buffer too small")\ + )) \ + _GL_CONCAT(FUNC,_chk_warn) (TYPE n, char *s, size_t size) \ + __asm__ (_GL_ASM_SYMBOL_PREFIX _GL_STRINGIFY(FUNC) "_chk"); \ + \ + static inline __attribute__ ((__always_inline__)) char * \ + _GL_CONCAT(FUNC,_chk_inline) (TYPE n, char *s, size_t size) \ + { \ + if (__builtin_constant_p (size)) \ + { \ + if (size != (size_t)-1) \ + { \ + /* buffer size known at compile time. */ \ + if (size >= INT_BUFSIZE_BOUND (TYPE)) \ + return (FUNC) (n, s); \ + else \ + return _GL_CONCAT(FUNC,_chk_warn) (n, s, size); \ + } \ + else \ + /* buffer size unknown. */ \ + return (FUNC) (n, s); \ + } \ + /* buffer size unknown at compile time but known at run time. */ \ + return _GL_CONCAT(FUNC,_chk) (n, s, size); \ + } + +_GL_DEFINE_INTTOSTR_FUNCS (imaxtostr, intmax_t) +# define imaxtostr(n, s) \ + (__extension__ (imaxtostr_chk_inline (n, s, _GL_pointed_object_size (s)))) + +_GL_DEFINE_INTTOSTR_FUNCS (inttostr, int) +# define inttostr(n, s) \ + (__extension__ (inttostr_chk_inline (n, s, _GL_pointed_object_size (s)))) + +_GL_DEFINE_INTTOSTR_FUNCS (offtostr, off_t) +# define offtostr(n, s) \ + (__extension__ (offtostr_chk_inline (n, s, _GL_pointed_object_size (s)))) + +_GL_DEFINE_INTTOSTR_FUNCS (uinttostr, unsigned int) +# define uinttostr(n, s) \ + (__extension__ (uinttostr_chk_inline (n, s, _GL_pointed_object_size (s)))) + +_GL_DEFINE_INTTOSTR_FUNCS (umaxtostr, uintmax_t) +# define umaxtostr(n, s) \ + (__extension__ (umaxtostr_chk_inline (n, s, _GL_pointed_object_size (s)))) + +#endif --- lib/verify.h.orig Sun Oct 17 21:54:43 2010 +++ lib/verify.h Sun Oct 17 21:33:55 2010 @@ -122,9 +122,11 @@ * In C++, any struct definition inside sizeof is invalid. Use a template type to work around the problem. */ +# ifndef _GL_CONCAT /* Concatenate two preprocessor tokens. */ -# define _GL_CONCAT(x, y) _GL_CONCAT0 (x, y) -# define _GL_CONCAT0(x, y) x##y +# define _GL_CONCAT(x, y) _GL_CONCAT0 (x, y) +# define _GL_CONCAT0(x, y) x##y +# endif /* _GL_COUNTER is an integer, preferably one that changes each time we use it. Use __COUNTER__ if it works, falling back on __LINE__ --- m4/inttostr.m4.orig Sun Oct 17 21:54:43 2010 +++ m4/inttostr.m4 Sun Oct 17 20:53:23 2010 @@ -1,4 +1,4 @@ -#serial 8 +#serial 9 dnl Copyright (C) 2004, 2005, 2006, 2009, 2010 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -16,6 +16,7 @@ # Prerequisites of lib/inttostr.h. AC_DEFUN([gl_PREREQ_INTTOSTR], [ AC_REQUIRE([AC_TYPE_OFF_T]) + AC_REQUIRE([gl_ASM_SYMBOL_PREFIX]) : ]) --- modules/inttostr.orig Sun Oct 17 21:54:43 2010 +++ modules/inttostr Sun Oct 17 20:52:40 2010 @@ -10,6 +10,7 @@ lib/umaxtostr.c lib/uinttostr.c m4/inttostr.m4 +m4/asm-underscore.m4 Depends-on: intprops --- modules/inttostr-tests.orig Sun Oct 17 21:54:43 2010 +++ modules/inttostr-tests Sun Oct 17 21:29:19 2010 @@ -1,6 +1,7 @@ Files: tests/macros.h tests/test-inttostr.c +tests/test-inttostr2.c Depends-on: intprops =========================== tests/test-inttostr2.c =========================== /* Test Fortify support for inttostr functions. Copyright (C) 2010 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation; either version 3 of the License, or (at your option) any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. You should have received a copy of the GNU General Public License along with this program. If not, see <http://www.gnu.org/licenses/>. */ /* Compile this with a GCC >= 4.3 on a glibc system, with options -O -S. */ /* Defining _FORTIFY_SOURCE, together with option -O, on a glibc system leads to a definition of __USE_FORTIFY_LEVEL. */ #undef _FORTIFY_SOURCE #define _FORTIFY_SOURCE 1 #include <config.h> #include "inttostr.h" #include <string.h> /* Buffer too small: warning at compile time, and call inttostr_chk. */ char * test_small_1 (int n) { char buf[5]; return strdup (inttostr (n, buf)); /* expect warning here */ } char * test_small_2 (int n) { char buf[5]; return strdup (inttostr (n, &buf[0])); /* expect warning here */ } /* Buffer large enough: no warning, and call inttostr. */ char * test_large_1 (int n) { char buf[12]; return strdup (inttostr (n, buf)); } char * test_large_2 (int n) { char buf[12]; return strdup (inttostr (n, &buf[0])); } /* Buffer size unknown at compile time but known at run time. No warning, but call inttostr_chk. */ char * test_variable_length_array (int n) { char buf[n < 0 ? 21 : 20]; return strdup (inttostr (n, buf)); } /* Buffer size unknown. No warning. Call inttostr. */ char * test_variable_length_array_pointer (int n) { char buf[n < 0 ? 21 : 20]; return strdup (inttostr (n, &buf[0])); } /* * For Emacs M-x compile * Local Variables: * compile-command: "gcc -I. -I.. -I../gllib -O -S test-inttostr2.c" * End: */