GNU bug report logs - #7347
[PATCH] stat: do not rely on undefined behavior in printf formats

Previous Next

Full log

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-coreutils <at> gnu.org
Subject: [PATCH] stat: do not rely on undefined behavior in printf formats
Date: Sat, 06 Nov 2010 14:03:44 -0700

I have not pushed this, as I understand you're trying to put out a
release, but this fixes some portability bugs in 'stat', such that it
was relying on undefined behavior, which presumably could cause
'stat' to dump core, or worse, on non-GNU platforms.

The downside of this patch is that the "I" printf flag is now ignored.
However, support for "I" wasn't working anyway (because of time stamp
fractions), so this isn't much of a loss.  We can add proper "I"
support later, if there's demand for it.

From 844d06f2f74cc26e648c9951f9ab43ae11c1dfc9 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert <at> cs.ucla.edu>
Date: Sat, 6 Nov 2010 13:57:08 -0700
Subject: [PATCH] stat: do not rely on undefined behavior in printf formats

* src/stat.c (digits, printf_flags): New static vars.
(make_format): New function.
(out_string, out_int, out_uint, out_uint_o, out_uint_x):
(out_minus_zero): Use it to avoid undefined behavior when invoking
printf.
(print_it): Check for invalid conversion specifications such as
%..X and %1-X, which would otherwise rely on undefined behavior
when invoking printf.
* tests/misc/stat-nanoseconds: Check that the "I" printf flag
doesn't mess up in the C locale, as it formerly did on non-GNU
hosts.
---
 src/stat.c                  |   47 ++++++++++++++++++++++++++++++++++++------
 tests/misc/stat-nanoseconds |    1 +
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/src/stat.c b/src/stat.c
index 99f115b..ae7ce02 100644
--- a/src/stat.c
+++ b/src/stat.c
@@ -150,6 +150,16 @@ statfs (char const *filename, struct fs_info *buf)
 #define hextobin(c) ((c) >= 'a' && (c) <= 'f' ? (c) - 'a' + 10 : \
                      (c) >= 'A' && (c) <= 'F' ? (c) - 'A' + 10 : (c) - '0')
 
+static char const digits[] = "0123456789";
+
+/* Flags that are portable for use in printf, for at least one
+   conversion specifier; make_format removes unportable flags as
+   needed for particular specifiers.  The glibc 2.2 extension "I" is
+   listed here; it is removed by make_format because it has undefined
+   behavior elsewhere and because it is incompatible with
+   out_epoch_sec.  */
+static char const printf_flags[] = "'-+ #0I";
+
 #define PROGRAM_NAME "stat"
 
 #define AUTHORS proper_name ("Michael Meskes")
@@ -467,40 +477,59 @@ human_time (struct timespec t)
   return str;
 }
 
+/* PFORMAT points to a '%' followed by a prefix of a format, all of
+   size PREFIX_LEN.  The flags allowed for this format are
+   ALLOWED_FLAGS; remove other printf flags from the prefix, then
+   append SUFFIX.  */
+static void
+make_format (char *pformat, size_t prefix_len, char const *allowed_flags,
+             char const *suffix)
+{
+  char *dst = pformat + 1;
+  char const *src;
+  char const *srclim = pformat + prefix_len;
+  for (src = dst; src < srclim && strchr (printf_flags, *src); src++)
+    if (strchr (allowed_flags, *src))
+      *dst++ = *src;
+  while (src < srclim)
+    *dst++ = *src++;
+  strcpy (dst, suffix);
+}
+
 static void
 out_string (char *pformat, size_t prefix_len, char const *arg)
 {
-  strcpy (pformat + prefix_len, "s");
+  make_format (pformat, prefix_len, "-", "s");
   printf (pformat, arg);
 }
 static int
 out_int (char *pformat, size_t prefix_len, intmax_t arg)
 {
-  strcpy (pformat + prefix_len, PRIdMAX);
+  make_format (pformat, prefix_len, "'-+ 0", PRIdMAX);
   return printf (pformat, arg);
 }
 static int
 out_uint (char *pformat, size_t prefix_len, uintmax_t arg)
 {
-  strcpy (pformat + prefix_len, PRIuMAX);
+  make_format (pformat, prefix_len, "'-0", PRIuMAX);
   return printf (pformat, arg);
 }
 static void
 out_uint_o (char *pformat, size_t prefix_len, uintmax_t arg)
 {
-  strcpy (pformat + prefix_len, PRIoMAX);
+  make_format (pformat, prefix_len, "-#0", PRIoMAX);
   printf (pformat, arg);
 }
 static void
 out_uint_x (char *pformat, size_t prefix_len, uintmax_t arg)
 {
-  strcpy (pformat + prefix_len, PRIxMAX);
+  make_format (pformat, prefix_len, "-#0", PRIxMAX);
   printf (pformat, arg);
 }
 static int
 out_minus_zero (char *pformat, size_t prefix_len)
 {
-  strcpy (pformat + prefix_len, ".0f");
+  make_format (pformat, prefix_len, "'-+ 0", ".0f");
   return printf (pformat, -0.25);
 }
 
@@ -1028,8 +1057,12 @@ print_it (char const *format, char const *filename,
         {
         case '%':
           {
-            size_t len = strspn (b + 1, "#-+.I 0123456789");
+            size_t len = strspn (b + 1, printf_flags);
             char const *fmt_char = b + len + 1;
+            fmt_char += strspn (fmt_char, digits);
+            if (*fmt_char == '.')
+              fmt_char += 1 + strspn (fmt_char + 1, digits);
+            len = fmt_char - (b + 1);
             unsigned int fmt_code = *fmt_char;
             memcpy (dest, b, len + 1);
 
diff --git a/tests/misc/stat-nanoseconds b/tests/misc/stat-nanoseconds
index 0f41eb0..cd21596 100755
--- a/tests/misc/stat-nanoseconds
+++ b/tests/misc/stat-nanoseconds
@@ -39,6 +39,7 @@ test "$(stat -c   %13.6X k)" =  ' 67413.023456'       || fail=1
 test "$(stat -c  %013.6X k)" =   067413.023456        || fail=1
 test "$(stat -c  %-13.6X k)" =   '67413.023456 '      || fail=1
 test "$(stat -c  %18.10X k)" = '  67413.0234567890'   || fail=1
+test "$(stat -c %I18.10X k)" = '  67413.0234567890'   || fail=1
 test "$(stat -c %018.10X k)" =  0067413.0234567890    || fail=1
 test "$(stat -c %-18.10X k)" =   '67413.0234567890  ' || fail=1
 
-- 
1.7.2

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.