* libamu/xutil.c (xsnprintf): if vsnprintf truncated the output
authorErez Zadok <ezk@cs.sunysb.edu>
Sun, 7 Aug 2005 01:25:33 +0000 (01:25 +0000)
committerErez Zadok <ezk@cs.sunysb.edu>
Sun, 7 Aug 2005 01:25:33 +0000 (01:25 +0000)
string to avoid an overflow, print an error.  Include some code to
break out any possible infinite loop between plog() and
xsnprintf().
(real_plog): now we can use (carefully) xsnprintf() directly.

ChangeLog
libamu/strutil.c
libamu/xutil.c

index 0f0c9b3de85dca77897f2b13441c98b508b94c55..2d0c92cb2bb5dee0bec364d9af045ee803374bcd 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2005-08-06  Erez Zadok  <ezk@cs.sunysb.edu>
 
+       * libamu/xutil.c (xsnprintf): if vsnprintf truncated the output
+       string to avoid an overflow, print an error.  Include some code to
+       break out any possible infinite loop between plog() and
+       xsnprintf().
+       (real_plog): now we can use (carefully) xsnprintf() directly.
+
        * amd/sun_map.[hc]: cleanup and formatting.
 
        * amd/sun_map_parse.y: to match the literal string "fstype=" use
index 59dee3ab62f2b9dcb1d0103aba486b9857c584aa..63966ec300af5f44dbc1040141dcbe0a87c9e07f 100644 (file)
@@ -99,6 +99,7 @@ xstrlcpy(char *dst, const char *src, size_t len)
     plog(XLOG_ERROR, "xstrlcpy: string \"%s\" truncated to \"%s\"", src, dst);
 }
 
+
 /*
  * Use generic strlcat to concatenate a string more carefully,
  * null-terminating it as needed.  However, if the copied string was
index 51837cbef3dc71672fa7480e6a595409c4cc0aa0..4b6b214eb620aa2894039006d3db043da8ca7756 100644 (file)
@@ -438,23 +438,12 @@ real_plog(int lvl, const char *fmt, va_list vargs)
 # endif /* not defined(HAVE_MALLINFO) && defined(HAVE_MALLOC_VERIFY) */
 #endif /* DEBUG_MEM */
 
-#ifdef HAVE_VSNPRINTF
-  /*
-   * XXX: ptr is 1024 bytes long, but we may write to ptr[strlen(ptr) + 2]
-   * (to add an '\n', see code below) so we have to limit the string copy
-   * to 1023 (including the '\0').
-   */
-  vsnprintf(ptr, 1023, expand_error(fmt, efmt, 1024), vargs);
-  msg[1022] = '\0';            /* null terminate, to be sure */
-#else /* not HAVE_VSNPRINTF */
   /*
-   * XXX: ptr is 1024 bytes long.  It is possible to write into it
-   * more than 1024 bytes, if efmt is already large, and vargs expand
-   * as well.  This is not as safe as using vsnprintf().
+   * Note: xsnprintf() may call plog() if a truncation happened, but the
+   * latter has some code to break out of an infinite loop.  See comment in
+   * xsnprintf() below.
    */
-  vsprintf(ptr, expand_error(fmt, efmt, 1023), vargs);
-  msg[1023] = '\0';            /* null terminate, to be sure */
-#endif /* not HAVE_VSNPRINTF */
+  xsnprintf(ptr, 1023, expand_error(fmt, efmt, 1024), vargs);
 
   ptr += strlen(ptr);
   if (*(ptr-1) == '\n')
@@ -969,7 +958,7 @@ int
 xsnprintf(char *str, size_t size, const char *format, ...)
 {
   va_list ap;
-  int ret;
+  int ret = 0;
 
   va_start(ap, format);
 #ifdef HAVE_VSNPRINTF
@@ -978,6 +967,19 @@ xsnprintf(char *str, size_t size, const char *format, ...)
   ret = vsprintf(str, format, ap); /* less secure version */
 #endif /* not HAVE_VSNPRINTF */
   va_end(ap);
+  /*
+   * If error or truncation, plog error.
+   *
+   * WARNING: we use the static 'maxtrunc' variable below to break out any
+   * possible infinite recursion between plog() and xsnprintf().  If it ever
+   * happens, it'd indicate a bug in Amd.
+   */
+  if (ret < 0 || ret >= size) {        /* error or truncation occured */
+    static int maxtrunc;       /* hack to avoid inifinite loop */
+    if (++maxtrunc > 10)
+      plog(XLOG_ERROR, "BUG: string %p truncated (ret=%d, format=\"%s\")",
+          str, ret, format);
+  }
   return ret;
 }
 
@@ -998,5 +1000,3 @@ setup_sighandler(int signum, void (*handler)(int))
   (void) signal(signum, handler);
 #endif /* not HAVE_SIGACTION */
 }
-
-