From fd20c4e799e7e43fc0f46c39bed62ade4d143f55 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Tue, 12 May 2009 08:35:17 +0000 Subject: [PATCH] logsys.c: avoid possibility of buffer overrun * exec/logsys.c (strcpy_cutoff): Add buf_len parameter, and never write more than this number of bytes into "dest". Change type of "cutoff" parameter from int to size_t. Sentinel value changes from -1 to 0. (log_printf_to_logs): Adapt to those changes. Reverse condition of test so the much-shorter block is the "if-block". Factor out a common subexpression for readability. Exit the loop if output_buffer_idx ever reaches sizeof(output_buffer). git-svn-id: http://svn.fedorahosted.org/svn/corosync/trunk@2178 fd59a12c-fef9-0310-b244-a6a79926bd2f --- exec/logsys.c | 49 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/exec/logsys.c b/exec/logsys.c index 5e977412..72ae9fa9 100644 --- a/exec/logsys.c +++ b/exec/logsys.c @@ -381,18 +381,26 @@ do { \ /* * Internal threaded logging implementation */ -static inline int strcpy_cutoff (char *dest, const char *src, int cutoff) +static inline int strcpy_cutoff (char *dest, const char *src, size_t cutoff, + size_t buf_len) { size_t len = strlen (src); - if (cutoff <= 0) { - memcpy (dest, src, len + 1); - return (len); - } else { - len = MIN (len, cutoff); - memcpy (dest, src, len); - memset (dest + len, ' ', cutoff - len); - dest[cutoff] = '\0'; + if (buf_len <= 1) { + if (buf_len == 0) + dest[0] = 0; + return 0; } + + if (cutoff == 0) { + cutoff = len; + } + + cutoff = MIN (cutoff, buf_len - 1); + len = MIN (len, cutoff); + memcpy (dest, src, len); + memset (dest + len, ' ', cutoff - len); + dest[cutoff] = '\0'; + return (cutoff); } @@ -421,7 +429,7 @@ static void log_printf_to_logs ( unsigned int format_buffer_idx = 0; unsigned int output_buffer_idx = 0; struct timeval tv; - int cutoff; + size_t cutoff; unsigned int len; int subsysid; @@ -430,9 +438,13 @@ static void log_printf_to_logs ( return; } - while (format_buffer[format_buffer_idx]) { - cutoff = -1; - if (format_buffer[format_buffer_idx] == '%') { + int c; + while ((c = format_buffer[format_buffer_idx])) { + cutoff = 0; + if (c != '%') { + output_buffer[output_buffer_idx++] = c; + format_buffer_idx++; + } else { const char *p; format_buffer_idx += 1; @@ -476,12 +488,15 @@ static void log_printf_to_logs ( p = ""; break; } - len = strcpy_cutoff (&output_buffer[output_buffer_idx], - p, cutoff); + len = strcpy_cutoff (output_buffer + output_buffer_idx, + p, cutoff, + (sizeof (output_buffer) + - output_buffer_idx)); output_buffer_idx += len; format_buffer_idx += 1; - } else { - output_buffer[output_buffer_idx++] = format_buffer[format_buffer_idx++]; + } + if (output_buffer_idx == sizeof (output_buffer)) { + break; } }