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

[Florian Weimer <fweimer at redhat dot com>]

This patch introduces a new failure mode for snprintf and similar 
functions.  If the buffer is implausibly large, we abort.

I think this is desirable because it covers a fairly common error in 
size computations for snprintf, like the one fixed in Exim.

   if (rr != NULL) {
     int rr_offset = 0;
     int answer_offset = 0;
     while (rr_offset < rr->size) {
       uschar len = (rr->data)[rr_offset++];
       snprintf(answer+(answer_offset),
                PDKIM_DNS_TXT_MAX_RECLEN-(answer_offset),
                "%.*s", (int)len, (char *)((rr->data)+rr_offset));
       rr_offset+=len;
       answer_offset+=len;
+      if (answer_offset >= PDKIM_DNS_TXT_MAX_RECLEN) {
+        return PDKIM_FAIL;
+      }
     }
   }

<http://git.exim.org/exim.git/commitdiff/4263f395efd136dece52d765dfcff3c96f17506e>

Tested on x86_64-redhat-linux-gnu.

--
Florian Weimer / Red Hat Product Security Team

2013-10-18  Florian Weimer  <fweimer@redhat.com>

	[BZ #14771]
	* debug/fortify.h: New file.
	* debug/Makefile (headers): Add fortify.h.
	* debug/tst-chk1.c (do_test): Add tests to exercise additional
	length check.
	* debug/vsnprintf_chk.c (___vsnprintf_chk): Replace
	user-supplied/compiler-provided length check with call to
	__check_buffer_length_int_max.
	* debug/vswprintf_chk.c (__vswprintf_chk): Likewise.

diff --git a/NEWS b/NEWS
index 0efdde5..68c136e 100644
--- a/NEWS
+++ b/NEWS
@@ -9,14 +9,14 @@ Version 2.19
 
 * The following bugs are resolved with this release:
 
-  156, 431, 832, 13028, 13982, 13985, 14155, 14547, 14699, 14910, 15048,
-  15218, 15277, 15308, 15362, 15400, 15427, 15522, 15531, 15532, 15608,
-  15609, 15610, 15632, 15640, 15672, 15680, 15681, 15723, 15734, 15735,
-  15736, 15748, 15749, 15754, 15760, 15764, 15797, 15844, 15847, 15849,
-  15855, 15856, 15857, 15859, 15867, 15886, 15887, 15890, 15892, 15893,
-  15895, 15897, 15905, 15909, 15919, 15921, 15923, 15939, 15963, 15966,
-  15895, 15897, 15905, 15909, 15919, 15921, 15923, 15939, 15963, 15966,
-  15988, 16032, 16034, 16036, 16041.
+  156, 431, 832, 13028, 13982, 13985, 14155, 14547, 14699, 14771, 14910,
+  15048, 15218, 15277, 15308, 15362, 15400, 15427, 15522, 15531, 15532,
+  15608, 15609, 15610, 15632, 15640, 15672, 15680, 15681, 15723, 15734,
+  15735, 15736, 15748, 15749, 15754, 15760, 15764, 15797, 15844, 15847,
+  15849, 15855, 15856, 15857, 15859, 15867, 15886, 15887, 15890, 15892,
+  15893, 15895, 15897, 15905, 15909, 15919, 15921, 15923, 15939, 15963,
+  15966, 15895, 15897, 15905, 15909, 15919, 15921, 15923, 15939, 15963,
+  15966, 15988, 16032, 16034, 16036, 16041.
 
 * CVE-2012-4412 The strcoll implementation caches indices and rules for
   large collation sequences to optimize multiple passes.  This cache
@@ -70,6 +70,10 @@ Version 2.19
 * SystemTap probes for malloc have been introduced.
 
 * Support for powerpc64le has been added.
+
+* In _FORTIFY_SOURCE=2 mode, snprintf, vsnprintf, swprintf, vswprintf will
+  abort if the user-supplied length argument is unusually large (close to
+  INT_MAX).
 
 Version 2.18
 
diff --git a/debug/Makefile b/debug/Makefile
index 13ee5c8..32f0883 100644
--- a/debug/Makefile
+++ b/debug/Makefile
@@ -20,7 +20,7 @@
 #
 subdir	:= debug
 
-headers	:= execinfo.h
+headers	:= execinfo.h fortify.h
 
 # Note that ptsname_r_chk and getlogin_r are not here, but in
 # login/Makefile instead.  If that subdir is omitted from the
diff --git a/debug/fortify.h b/debug/fortify.h
new file mode 100644
index 0000000..725424d
--- /dev/null
+++ b/debug/fortify.h
@@ -0,0 +1,43 @@
+/* Copyright (C) 2004-2013 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#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 ();
+}
+
+__END_DECLS
+
+
+#endif /* fortify.h  */
diff --git a/debug/tst-chk1.c b/debug/tst-chk1.c
index 6ca8d9d..2b1da18 100644
--- a/debug/tst-chk1.c
+++ b/debug/tst-chk1.c
@@ -398,6 +398,17 @@ do_test (void)
   CHK_FAIL_END
 #endif
 
+# if __USE_FORTIFY_LEVEL >= 2
+  p = (char *) 1;
+  CHK_FAIL_START
+  snprintf ((char *) p, -1, "%d", 1);
+  CHK_FAIL_END
+
+  p = (char *)1;
+  CHK_FAIL_START
+  snprintf ((char *) p, INT_MAX - 17, "%d", 1);
+  CHK_FAIL_END
+# endif
 
   /* These ops can be done without runtime checking of object size.  */
   wmemcpy (wbuf, L"abcdefghij", 10);
diff --git a/debug/vsnprintf_chk.c b/debug/vsnprintf_chk.c
index 8924f99..b28d85d 100644
--- a/debug/vsnprintf_chk.c
+++ b/debug/vsnprintf_chk.c
@@ -19,6 +19,7 @@
 #include <stdio.h>
 #include "../libio/libioP.h"
 #include "../libio/strfile.h"
+#include "fortify.h"
 
 extern const struct _IO_jump_t _IO_strn_jumps attribute_hidden;
 
@@ -29,12 +30,7 @@ int
 ___vsnprintf_chk (char *s, size_t maxlen, int flags, size_t slen,
 		  const char *format, va_list args)
 {
-  /* XXX Maybe for less strict version do not fail immediately.
-     Though, maxlen is supposed to be the size of buffer pointed
-     to by s, so a conforming program can't pass such maxlen
-     to *snprintf.  */
-  if (__builtin_expect (slen < maxlen, 0))
-    __chk_fail ();
+  __check_buffer_length_int_max (maxlen, slen);
 
   _IO_strnfile sf;
   int ret;
diff --git a/debug/vswprintf_chk.c b/debug/vswprintf_chk.c
index b298a0b..c14db6d 100644
--- a/debug/vswprintf_chk.c
+++ b/debug/vswprintf_chk.c
@@ -19,6 +19,7 @@
 #include <wchar.h>
 #include "../libio/libioP.h"
 #include "../libio/strfile.h"
+#include "fortify.h"
 
 
 /* Write formatted output into S, according to the format
@@ -28,12 +29,7 @@ int
 __vswprintf_chk (wchar_t *s, size_t maxlen, int flags, size_t slen,
 		 const wchar_t *format, va_list args)
 {
-  /* XXX Maybe for less strict version do not fail immediately.
-     Though, maxlen is supposed to be the size of buffer pointed
-     to by s, so a conforming program can't pass such maxlen
-     to *snprintf.  */
-  if (__builtin_expect (slen < maxlen, 0))
-    __chk_fail ();
+  __check_buffer_length_int_max (maxlen, slen);
 
   _IO_wstrnfile sf;
   struct _IO_wide_data wd;