stdio: warn on suspicious uses

Using gets is almost ALWAYS wrong (it is extremely rare that you have
full control over stdin).  POSIX 2008 marked it as obsolete, even
though C89 requires it.  Attach a warning to remind developers.  Add
a comment to sprintf explaining why it does not get this treatment.

Improve the warnings for fseek/fseeko (and ftell/ftello), with
comments justifying our position.

Some of our unit tests never use large files, so rather than drag
in a dependency on fseeko, they should be the first compilation
units to use _GL_NO_LARGE_FILES.

* modules/stdio (Depends-on): Add warn-on-use.
(Makefile.am): Provide new substitutions.
* m4/stdio_h.m4 (gl_STDIO_H): Check for inline, ftello, and
fseeko.
* lib/stdio.in.h (gets): Always warn on use.
(fseek, ftell): Adjust when warnings are issued, and honor
_GL_NO_LARGE_FILES as a way to silence the warning.
* tests/test-fpurge.c [!GNULIB_FSEEK]: Use new means to squelch
any warning about large file offsets.
* tests/test-freadable.c [!GNULIB_FSEEK]: Likewise.
* tests/test-freading.c [!GNULIB_FSEEK]: Likewise.
* tests/test-fseeko.c [!GNULIB_FSEEK]: Likewise.
* tests/test-ftell.c [!GNULIB_FSEEK]: Likewise.
* tests/test-ftello.c [!GNULIB_FSEEK]: Likewise.
* tests/test-fwritable.c [!GNULIB_FSEEK]: Likewise.
* tests/test-fwriting.c [!GNULIB_FSEEK]: Likewise.
* tests/test-getopt.c [!GNULIB_FTELL]: Likewise.

Signed-off-by: Eric Blake <ebb9@byu.net>

diff --git a/ChangeLog b/ChangeLog
index 08a43904cf9bf60fe9f58503aa7bb8d2f1d423ab..c898b9a09a6c72ad225afea721659e89f74f6bf5 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,24 @@
 2010-01-11  Eric Blake  <ebb9@byu.net>
 
+       stdio: warn on suspicious uses
+       * modules/stdio (Depends-on): Add warn-on-use.
+       (Makefile.am): Provide new substitutions.
+       * m4/stdio_h.m4 (gl_STDIO_H): Check for inline, ftello, and
+       fseeko.
+       * lib/stdio.in.h (gets): Always warn on use.
+       (fseek, ftell): Adjust when warnings are issued, and honor
+       _GL_NO_LARGE_FILES as a way to silence the warning.
+       * tests/test-fpurge.c [!GNULIB_FSEEK]: Use new means to squelch
+       any warning about large file offsets.
+       * tests/test-freadable.c [!GNULIB_FSEEK]: Likewise.
+       * tests/test-freading.c [!GNULIB_FSEEK]: Likewise.
+       * tests/test-fseeko.c [!GNULIB_FSEEK]: Likewise.
+       * tests/test-ftell.c [!GNULIB_FSEEK]: Likewise.
+       * tests/test-ftello.c [!GNULIB_FSEEK]: Likewise.
+       * tests/test-fwritable.c [!GNULIB_FSEEK]: Likewise.
+       * tests/test-fwriting.c [!GNULIB_FSEEK]: Likewise.
+       * tests/test-getopt.c [!GNULIB_FTELL]: Likewise.
+
        warn-on-use: new module
        * modules/warn-on-use: New file.
        * build-aux/warn-on-use.h: Likewise.

diff --git a/lib/stdio.in.h b/lib/stdio.in.h
index 9e1fac38efae6b7c6dabfdb364e814d53a0bcb1b..3f0ddae6b752b097f211fcf6a9b4d54b581e193c 100644 (file)
--- a/lib/stdio.in.h
+++ b/lib/stdio.in.h
@@ -62,6 +62,8 @@
 
 /* The definition of _GL_ARG_NONNULL is copied here.  */
 
+/* The definition of _GL_WARN_ON_USE is copied here.  */
+
 
 #ifdef __cplusplus
 extern "C" {
@@ -118,6 +120,12 @@ extern int fclose (FILE *stream) _GL_ARG_NONNULL ((1));
     fflush (f))
 #endif
 
+/* It is very rare that the developer ever has full control of stdin,
+   so any use of gets warrants an unconditional warning.  Assume it is
+   always declared, since it is required by C89.  */
+#undef gets
+_GL_WARN_ON_USE (gets, "gets is a security hole - use fgets instead");
+
 #if @GNULIB_FOPEN@
 # if @REPLACE_FOPEN@
 #  undef fopen
@@ -202,92 +210,136 @@ extern FILE * freopen (const char *filename, const char *mode, FILE *stream)
     freopen (f, m, s))
 #endif
 
-#if @GNULIB_FSEEK@ && @REPLACE_FSEEK@
-extern int rpl_fseek (FILE *fp, long offset, int whence) _GL_ARG_NONNULL ((1));
-# undef fseek
-# if defined GNULIB_POSIXCHECK
-#  define fseek(f,o,w) \
-     (GL_LINK_WARNING ("fseek cannot handle files larger than 4 GB " \
-                       "on 32-bit platforms - " \
-                       "use fseeko function for handling of large files"), \
-      rpl_fseek (f, o, w))
-# else
+/* Set up the following warnings, based on which modules are in use.
+   GNU Coding Standards discourage the use of fseek, since it imposes
+   an arbitrary limitation on some 32-bit hosts.  Remember that the
+   fseek module depends on the fseeko module, so we only have three
+   cases to consider:
+
+   1. The developer is not using either module.  Issue a warning under
+   GNULIB_POSIXCHECK for both functions, to remind them that both
+   functions have bugs on some systems.  _GL_NO_LARGE_FILES has no
+   impact on this warning.
+
+   2. The developer is using both modules.  They may be unaware of the
+   arbitrary limitations of fseek, so issue a warning under
+   GNULIB_POSIXCHECK.  On the other hand, they may be using both
+   modules intentionally, so the developer can define
+   _GL_NO_LARGE_FILES in the compilation units where the use of fseek
+   is safe, to silence the warning.
+
+   3. The developer is using the fseeko module, but not fseek.  Gnulib
+   guarantees that fseek will still work around platform bugs in that
+   case, but we presume that the developer is aware of the pitfalls of
+   fseek and was trying to avoid it, so issue a warning even when
+   GNULIB_POSIXCHECK is undefined.  Again, _GL_NO_LARGE_FILES can be
+   defined to silence the warning in particular compilation units.
+
+   Most gnulib clients that perform stream operations should fall into
+   category three.  */
+
+#if @GNULIB_FSEEK@
+# if defined GNULIB_POSIXCHECK && !defined _GL_NO_LARGE_FILES
+#  define _GL_FSEEK_WARN /* Category 2, above.  */
+#  undef fseek
+# endif
+# if @REPLACE_FSEEK@
+#  undef fseek
 #  define fseek rpl_fseek
-# endif
-#elif defined GNULIB_POSIXCHECK
-# ifndef fseek
-#  define fseek(f,o,w) \
-     (GL_LINK_WARNING ("fseek cannot handle files larger than 4 GB " \
-                       "on 32-bit platforms - " \
-                       "use fseeko function for handling of large files"), \
-      fseek (f, o, w))
+extern int fseek (FILE *fp, long offset, int whence) _GL_ARG_NONNULL ((1));
 # endif
 #endif
 
 #if @GNULIB_FSEEKO@
+# if !@GNULIB_FSEEK@ && !defined _GL_NO_LARGE_FILES
+#  define _GL_FSEEK_WARN /* Category 3, above.  */
+#  undef fseek
+# endif
 # if @REPLACE_FSEEKO@
 /* Provide fseek, fseeko functions that are aware of a preceding
    fflush(), and which detect pipes.  */
+#  undef fseeko
 #  define fseeko rpl_fseeko
 extern int fseeko (FILE *fp, off_t offset, int whence) _GL_ARG_NONNULL ((1));
 #  if !@GNULIB_FSEEK@
 #   undef fseek
-#   define fseek(f,o,w) \
-     (GL_LINK_WARNING ("fseek cannot handle files larger than 4 GB " \
-                       "on 32-bit platforms - " \
-                       "use fseeko function for handling of large files"), \
-      fseeko (f, o, w))
+#   define fseek rpl_fseek
+static inline int _GL_ARG_NONNULL ((1))
+rpl_fseek (FILE *fp, long offset, int whence)
+{
+  return fseeko (fp, offset, whence);
+}
 #  endif
 # endif
 #elif defined GNULIB_POSIXCHECK
+# define _GL_FSEEK_WARN /* Category 1, above.  */
+# undef fseek
 # undef fseeko
-# define fseeko(f,o,w) \
-   (GL_LINK_WARNING ("fseeko is unportable - " \
-                     "use gnulib module fseeko for portability"), \
-    fseeko (f, o, w))
+# if HAVE_RAW_DECL_FSEEKO
+_GL_WARN_ON_USE (fseeko, "fseeko is unportable - "
+                 "use gnulib module fseeko for portability");
+# endif
 #endif
 
-#if @GNULIB_FTELL@ && @REPLACE_FTELL@
-extern long rpl_ftell (FILE *fp) _GL_ARG_NONNULL ((1));
-# undef ftell
-# if GNULIB_POSIXCHECK
-#  define ftell(f) \
-     (GL_LINK_WARNING ("ftell cannot handle files larger than 4 GB " \
-                       "on 32-bit platforms - " \
-                       "use ftello function for handling of large files"), \
-      rpl_ftell (f))
-# else
-#  define ftell rpl_ftell
+#ifdef _GL_FSEEK_WARN
+# undef _GL_FSEEK_WARN
+/* Here, either fseek is undefined (but C89 guarantees that it is
+   declared), or it is defined as rpl_fseek (declared above).  */
+_GL_WARN_ON_USE (fseek, "fseek cannot handle files larger than 4 GB "
+                 "on 32-bit platforms - "
+                 "use fseeko function for handling of large files");
+#endif
+
+/* See the comments on fseek/fseeko.  */
+
+#if @GNULIB_FTELL@
+# if defined GNULIB_POSIXCHECK && !defined _GL_NO_LARGE_FILES
+#  define _GL_FTELL_WARN /* Category 2, above.  */
+#  undef ftell
 # endif
-#elif defined GNULIB_POSIXCHECK
-# ifndef ftell
-#  define ftell(f) \
-     (GL_LINK_WARNING ("ftell cannot handle files larger than 4 GB " \
-                       "on 32-bit platforms - " \
-                       "use ftello function for handling of large files"), \
-      ftell (f))
+# if && @REPLACE_FTELL@
+#  undef ftell
+#  define ftell rpl_ftell
+extern long ftell (FILE *fp) _GL_ARG_NONNULL ((1));
 # endif
 #endif
 
 #if @GNULIB_FTELLO@
+# if !@GNULIB_FTELL@ && !defined _GL_NO_LARGE_FILES
+#  define _GL_FTELL_WARN /* Category 3, above.  */
+#  undef ftell
+# endif
 # if @REPLACE_FTELLO@
+#  undef ftello
 #  define ftello rpl_ftello
 extern off_t ftello (FILE *fp) _GL_ARG_NONNULL ((1));
 #  if !@GNULIB_FTELL@
 #   undef ftell
-#   define ftell(f) \
-     (GL_LINK_WARNING ("ftell cannot handle files larger than 4 GB " \
-                       "on 32-bit platforms - " \
-                       "use ftello function for handling of large files"), \
-      ftello (f))
+#   define ftell rpl_ftell
+static inline long _GL_ARG_NONNULL ((1))
+rpl_ftell (FILE *f)
+{
+  return ftello (f);
+}
 #  endif
 # endif
 #elif defined GNULIB_POSIXCHECK
+# define _GL_FTELL_WARN /* Category 1, above.  */
+# undef ftell
 # undef ftello
-# define ftello(f) \
-   (GL_LINK_WARNING ("ftello is unportable - " \
-                     "use gnulib module ftello for portability"), \
-    ftello (f))
+# if HAVE_RAW_DECL_FTELLO
+_GL_WARN_ON_USE (ftello, "ftello is unportable - "
+                 "use gnulib module ftello for portability");
+# endif
+#endif
+
+#ifdef _GL_FTELL_WARN
+# undef _GL_FTELL_WARN
+/* Here, either ftell is undefined (but C89 guarantees that it is
+   declared), or it is defined as rpl_ftell (declared above).  */
+_GL_WARN_ON_USE (ftell, "ftell cannot handle files larger than 4 GB "
+                 "on 32-bit platforms - "
+                 "use ftello function for handling of large files");
 #endif
 
 #if @GNULIB_FWRITE@ && @REPLACE_STDIO_WRITE_FUNCS@ && @GNULIB_STDIO_H_SIGPIPE@
@@ -500,6 +552,15 @@ extern int snprintf (char *str, size_t size, const char *format, ...)
      snprintf)
 #endif
 
+/* Some people would argue that sprintf should be handled like gets
+   (for example, OpenBSD issues a link warning for both functions),
+   since both can cause security holes due to buffer overruns.
+   However, we believe that sprintf can be used safely, and is more
+   efficient than snprintf in those safe cases; and as proof of our
+   belief, we use sprintf in several gnulib modules.  So this header
+   intentionally avoids adding a warning to sprintf except when
+   GNULIB_POSIXCHECK is defined.  */
+
 #if @GNULIB_SPRINTF_POSIX@
 # if @REPLACE_SPRINTF@
 #  define sprintf rpl_sprintf

diff --git a/m4/stdio_h.m4 b/m4/stdio_h.m4
index 7e596923aea96e33cd4f3437f2be35f8ac5589bf..be79896d5712a69383611f88f9d25125b0f95ce0 100644 (file)
--- a/m4/stdio_h.m4
+++ b/m4/stdio_h.m4
@@ -1,4 +1,4 @@
-# stdio_h.m4 serial 22
+# stdio_h.m4 serial 23
 dnl Copyright (C) 2007-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,
@@ -7,6 +7,7 @@ dnl with or without modifications, as long as this notice is preserved.
 AC_DEFUN([gl_STDIO_H],
 [
   AC_REQUIRE([gl_STDIO_H_DEFAULTS])
+  AC_REQUIRE([AC_C_INLINE])
   gl_CHECK_NEXT_HEADERS([stdio.h])
   dnl No need to create extra modules for these functions. Everyone who uses
   dnl <stdio.h> likely needs them.
@@ -30,6 +31,12 @@ AC_DEFUN([gl_STDIO_H],
       AC_LIBOBJ([stdio-write])
     fi
   ])
+
+  dnl Check for declarations of anything we want to poison if the
+  dnl corresponding gnulib module is not in use, and which is not
+  dnl guaranteed by C89.
+  gl_WARN_ON_USE_PREPARE([[#include <stdio.h>
+    ]], [fseeko ftello])
 ])
 
 AC_DEFUN([gl_STDIO_MODULE_INDICATOR],

diff --git a/modules/stdio b/modules/stdio
index eca35813c7752b5fd7fa68296c1b1a9699c0a75f..6da339bf362c0505c9efbadbac22164c50cc78bb 100644 (file)
--- a/modules/stdio
+++ b/modules/stdio
@@ -12,6 +12,7 @@ link-warning
 arg-nonnull
 raise
 stddef
+warn-on-use
 
 configure.ac:
 gl_STDIO_H
@@ -21,7 +22,7 @@ BUILT_SOURCES += stdio.h
 
 # We need the following in order to create <stdio.h> when the system
 # doesn't have one that works with the given compiler.
-stdio.h: stdio.in.h $(LINK_WARNING_H) $(ARG_NONNULL_H)
+stdio.h: stdio.in.h $(LINK_WARNING_H) $(WARN_ON_USE_H) $(ARG_NONNULL_H)
        $(AM_V_GEN)rm -f $@-t $@ && \
        { echo '/* DO NOT EDIT! GENERATED AUTOMATICALLY! */' && \
          sed -e 's|@''INCLUDE_NEXT''@|$(INCLUDE_NEXT)|g' \
@@ -107,6 +108,7 @@ stdio.h: stdio.in.h $(LINK_WARNING_H) $(ARG_NONNULL_H)
              -e 's|@''REPLACE_VSPRINTF''@|$(REPLACE_VSPRINTF)|g' \
              -e '/definition of GL_LINK_WARNING/r $(LINK_WARNING_H)' \
              -e '/definition of _GL_ARG_NONNULL/r $(ARG_NONNULL_H)' \
+             -e '/definition of _GL_WARN_ON_USE/r $(WARN_ON_USE_H)' \
              < $(srcdir)/stdio.in.h; \
        } > $@-t && \
        mv $@-t $@

diff --git a/tests/test-fpurge.c b/tests/test-fpurge.c
index 15eff65058406c7db8a733b0bafa5604dbb4a723..89fe8c0caf6be70369fb8a762d46f80d683e27c0 100644 (file)
--- a/tests/test-fpurge.c
+++ b/tests/test-fpurge.c
@@ -18,18 +18,15 @@
 
 #include <config.h>
 
+/* None of the files accessed by this test are large, so disable the
+   fseek link warning if we are not using the gnulib fseek module.  */
+#define _GL_NO_LARGE_FILES
 #include <stdio.h>
 
 #include <string.h>
 
 #include "macros.h"
 
-/* None of the files accessed by this test are large, so disable the
-   fseek link warning if we are not using the gnulib fseek module.  */
-#if !GNULIB_FSEEK
-# undef fseek
-#endif
-
 #define TESTFILE "t-fpurge.tmp"
 
 int

diff --git a/tests/test-freadable.c b/tests/test-freadable.c
index 8f337d88e60e0833e5f0077ea081feae9824064c..2aa6743c561699c3d647afc59e5f460e73ff585f 100644 (file)
--- a/tests/test-freadable.c
+++ b/tests/test-freadable.c
@@ -18,18 +18,15 @@
 
 #include <config.h>
 
+/* None of the files accessed by this test are large, so disable the
+   fseek link warning if we are not using the gnulib fseek module.  */
+#define _GL_NO_LARGE_FILES
 #include "freadable.h"
 
 #include <stdio.h>
 
 #include "macros.h"
 
-/* None of the files accessed by this test are large, so disable the
-   fseek link warning if we are not using the gnulib fseek module.  */
-#if !GNULIB_FSEEK
-# undef fseek
-#endif
-
 #define TESTFILE "t-freadable.tmp"
 
 int

diff --git a/tests/test-freading.c b/tests/test-freading.c
index c4c00f4571db70325ece4c9c12413a921b38d75d..fee82284d871100b22ed5428738fc295da82ea59 100644 (file)
--- a/tests/test-freading.c
+++ b/tests/test-freading.c
@@ -18,18 +18,15 @@
 
 #include <config.h>
 
+/* None of the files accessed by this test are large, so disable the
+   fseek link warning if we are not using the gnulib fseek module.  */
+#define _GL_NO_LARGE_FILES
 #include "freading.h"
 
 #include <stdio.h>
 
 #include "macros.h"
 
-/* None of the files accessed by this test are large, so disable the
-   fseek link warning if we are not using the gnulib fseek module.  */
-#if !GNULIB_FSEEK
-# undef fseek
-#endif
-
 #define TESTFILE "t-freading.tmp"
 
 int

diff --git a/tests/test-fseeko.c b/tests/test-fseeko.c
index e8c66dfe202f6c9faf0925d5ddf2e4bc4c3d10be..be2a78dce1b702e291637dece49ab522afa36637 100644 (file)
--- a/tests/test-fseeko.c
+++ b/tests/test-fseeko.c
@@ -20,11 +20,7 @@
 
 /* None of the files accessed by this test are large, so disable the
    fseek link warning if we are not using the gnulib fseek module.  */
-#if !GNULIB_FSEEK
-# undef GL_LINK_WARNING
-# define GL_LINK_WARNING(ignored) ((void) 0)
-#endif
-
+#define _GL_NO_LARGE_FILES
 #include <stdio.h>
 
 #include "signature.h"

diff --git a/tests/test-ftell.c b/tests/test-ftell.c
index 42eff9cdfb1008eed504efee0f896f0baafbaa1c..6ad8e0a99ad15dc7af11a63faf1585207283b6c7 100644 (file)
--- a/tests/test-ftell.c
+++ b/tests/test-ftell.c
@@ -18,6 +18,9 @@
 
 #include <config.h>
 
+/* None of the files accessed by this test are large, so disable the
+   fseek link warning if we are not using the gnulib fseek module.  */
+#define _GL_NO_LARGE_FILES
 #include <stdio.h>
 
 #include "signature.h"
@@ -26,12 +29,6 @@ SIGNATURE_CHECK (ftell, long, (FILE *));
 #include "binary-io.h"
 #include "macros.h"
 
-/* None of the files accessed by this test are large, so disable the
-   fseek link warning if we are not using the gnulib fseek module.  */
-#if !GNULIB_FSEEK
-# undef fseek
-#endif
-
 #ifndef FUNC_UNGETC_BROKEN
 # define FUNC_UNGETC_BROKEN 0
 #endif

diff --git a/tests/test-ftello.c b/tests/test-ftello.c
index 826df10360455d1d319e1626bc61781ae4d040b8..5fae570f2881b12a4b72f3dcc4a906f2e47dff40 100644 (file)
--- a/tests/test-ftello.c
+++ b/tests/test-ftello.c
@@ -18,6 +18,9 @@
 
 #include <config.h>
 
+/* None of the files accessed by this test are large, so disable the
+   fseek link warning if we are not using the gnulib fseek module.  */
+#define _GL_NO_LARGE_FILES
 #include <stdio.h>
 
 #include "signature.h"
@@ -26,12 +29,6 @@ SIGNATURE_CHECK (ftello, off_t, (FILE *));
 #include "binary-io.h"
 #include "macros.h"
 
-/* None of the files accessed by this test are large, so disable the
-   fseek link warning if we are not using the gnulib fseek module.  */
-#if !GNULIB_FSEEK
-# undef fseek
-#endif
-
 #ifndef FUNC_UNGETC_BROKEN
 # define FUNC_UNGETC_BROKEN 0
 #endif

diff --git a/tests/test-fwritable.c b/tests/test-fwritable.c
index 2d696e97fca751303c6c93b4f84e22148a5a751a..ab6ca95fd91fef97c861737f41d5012ef52ee9bf 100644 (file)
--- a/tests/test-fwritable.c
+++ b/tests/test-fwritable.c
@@ -18,18 +18,15 @@
 
 #include <config.h>
 
+/* None of the files accessed by this test are large, so disable the
+   fseek link warning if we are not using the gnulib fseek module.  */
+#define _GL_NO_LARGE_FILES
 #include "fwritable.h"
 
 #include <stdio.h>
 
 #include "macros.h"
 
-/* None of the files accessed by this test are large, so disable the
-   fseek link warning if we are not using the gnulib fseek module.  */
-#if !GNULIB_FSEEK
-# undef fseek
-#endif
-
 #define TESTFILE "t-fwritable.tmp"
 
 int

diff --git a/tests/test-fwriting.c b/tests/test-fwriting.c
index ccc302e1c4332148b5346f344f8df8f9ec42ee31..d44d0034aac1d2767e369a3479f9119932b75999 100644 (file)
--- a/tests/test-fwriting.c
+++ b/tests/test-fwriting.c
@@ -18,18 +18,15 @@
 
 #include <config.h>
 
+/* None of the files accessed by this test are large, so disable the
+   fseek link warning if we are not using the gnulib fseek module.  */
+#define _GL_NO_LARGE_FILES
 #include "fwriting.h"
 
 #include <stdio.h>
 
 #include "macros.h"
 
-/* None of the files accessed by this test are large, so disable the
-   fseek link warning if we are not using the gnulib fseek module.  */
-#if !GNULIB_FSEEK
-# undef fseek
-#endif
-
 #define TESTFILE "t-fwriting.tmp"
 
 int

diff --git a/tests/test-getopt.c b/tests/test-getopt.c
index fe822ed16d5a098c386c052f941b4b256d95bde3..6bcb8e6ae4a933ff69cfce66dced6ca7dd3a9db4 100644 (file)
--- a/tests/test-getopt.c
+++ b/tests/test-getopt.c
@@ -20,10 +20,7 @@
 
 /* None of the files accessed by this test are large, so disable the
    ftell link warning if we are not using the gnulib ftell module.  */
-#if !GNULIB_FTELL
-# undef GL_LINK_WARNING
-# define GL_LINK_WARNING(ignored) ((void) 0)
-#endif
+#define _GL_NO_LARGE_FILES
 
 #if GNULIB_GETOPT_GNU
 # include <getopt.h>