From 4751b00703853bf932c0e234450f87d9e4dfc0ea Mon Sep 17 00:00:00 2001 From: Erez Zadok Date: Sun, 7 Aug 2005 01:25:33 +0000 Subject: [PATCH] * 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. --- ChangeLog | 6 ++++++ libamu/strutil.c | 1 + libamu/xutil.c | 36 ++++++++++++++++++------------------ 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0f0c9b3..2d0c92c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2005-08-06 Erez Zadok + * 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 diff --git a/libamu/strutil.c b/libamu/strutil.c index 59dee3a..63966ec 100644 --- a/libamu/strutil.c +++ b/libamu/strutil.c @@ -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 diff --git a/libamu/xutil.c b/libamu/xutil.c index 51837cb..4b6b214 100644 --- a/libamu/xutil.c +++ b/libamu/xutil.c @@ -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 */ } - - -- 2.43.0