From 20a3dee59732679705c8614b786b9d053434d180 Mon Sep 17 00:00:00 2001 From: mquinson Date: Tue, 11 Sep 2007 13:32:47 +0000 Subject: [PATCH] Get serious about checking buffer overflows during log construction git-svn-id: svn+ssh://scm.gforge.inria.fr/svn/simgrid/simgrid/trunk@4578 48e7efb5-ca39-0410-a469-dd3cf9ba447f --- include/xbt/log.h | 3 +- src/xbt/xbt_log_layout_format.c | 168 ++++++++++++++++++++------------ src/xbt/xbt_log_layout_simple.c | 28 ++++-- 3 files changed, 124 insertions(+), 75 deletions(-) diff --git a/include/xbt/log.h b/include/xbt/log.h index c28fbd0fa7..0a307f2d4e 100644 --- a/include/xbt/log.h +++ b/include/xbt/log.h @@ -234,6 +234,7 @@ typedef struct xbt_log_category_s s_xbt_log_category_t,*xbt_log_category_t; /* * Do NOT access any members of this structure directly. FIXME: move to private? */ +#define XBT_LOG_BUFF_SIZE 2048 /* Size of the static string in which we build the log string */ struct xbt_log_category_s { xbt_log_category_t parent; xbt_log_category_t firstChild; @@ -267,7 +268,7 @@ struct xbt_log_event_s { const char* functionName; int lineNum; va_list ap; - char buffer[1024]; + char buffer[XBT_LOG_BUFF_SIZE]; }; /** diff --git a/src/xbt/xbt_log_layout_format.c b/src/xbt/xbt_log_layout_format.c index 2407d59cb9..c17e821c29 100644 --- a/src/xbt/xbt_log_layout_format.c +++ b/src/xbt/xbt_log_layout_format.c @@ -16,6 +16,13 @@ extern const char *xbt_log_priority_names[7]; +/* Get serious about checking buffer overflows during log construction */ +#define check_overflow \ + if (p-ev->buffer > XBT_LOG_BUFF_SIZE) { /* buffer overflow */ \ + p=ev->buffer + XBT_LOG_BUFF_SIZE - strlen(" >> OUTPUT TRUNCATED <<\n"); \ + p+=sprintf(p," >> OUTPUT TRUNCATED <<\n"); \ + return;\ + } static void xbt_log_layout_format_doit(xbt_log_layout_t l, xbt_log_event_t ev, @@ -42,10 +49,12 @@ static void xbt_log_layout_format_doit(xbt_log_layout_t l, *p++ = '%'; break; case 'n': /* platform-dependant line separator (LOG4J compliant) */ - p += sprintf(p,"\n"); + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer),"\n"); + check_overflow; break; case 'e': /* plain space (SimGrid extension) */ - p += sprintf(p," "); + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer)," "); + check_overflow; break; case '.': /* precision specifyier */ @@ -55,84 +64,104 @@ static void xbt_log_layout_format_doit(xbt_log_layout_t l, case 'c': /* category name; LOG4J compliant should accept a precision postfix to show the hierarchy */ - if (precision == -1) - p += sprintf(p,"%s",ev->cat->name); - else { - p += sprintf(p,"%.*s",precision,ev->cat->name); + if (precision == -1) { + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer),"%s",ev->cat->name); + check_overflow; + } else { + p += sprintf(p,"%.*s",MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision),ev->cat->name); + check_overflow; precision = -1; } break; case 'p': /* priority name; LOG4J compliant */ - if (precision == -1) - p += sprintf(p, "%s", xbt_log_priority_names[ev->priority] ); - else { - p += sprintf(p, "%.*s", precision, xbt_log_priority_names[ev->priority] ); + if (precision == -1) { + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer), "%s", xbt_log_priority_names[ev->priority] ); + check_overflow; + } else { + p += sprintf(p, "%.*s", MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision), xbt_log_priority_names[ev->priority] ); + check_overflow; precision = -1; } break; case 'h': /* host name; SimGrid extension */ - if (precision == -1) - p += sprintf(p, "%s", gras_os_myname()); - else { - p += sprintf(p, "%.*s", precision, gras_os_myname()); + if (precision == -1) { + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer), "%s", gras_os_myname()); + check_overflow; + } else { + p += sprintf(p, "%.*s", MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision), gras_os_myname()); + check_overflow; precision = -1; } break; case 't': /* thread name; LOG4J compliant */ - if (precision == -1) - p += sprintf(p, "%s", xbt_thread_self_name()); - else { - p += sprintf(p, "%.*s", precision, xbt_thread_self_name()); + if (precision == -1) { + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer), "%s", xbt_thread_self_name()); + check_overflow; + } else { + p += sprintf(p, "%.*s", MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision), xbt_thread_self_name()); + check_overflow; precision = -1; } break; case 'P': /* process name; SimGrid extension */ - if (precision == -1) - p += sprintf(p, "%s", xbt_procname()); - else { - p += sprintf(p, "%.*s", precision,xbt_procname()); + if (precision == -1) { + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer), "%s", xbt_procname()); + check_overflow; + } else { + p += sprintf(p, "%.*s", MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision),xbt_procname()); + check_overflow; precision = -1; } break; case 'i': /* process PID name; SimGrid extension */ - if (precision == -1) - p += sprintf(p, "%d", (*xbt_getpid)()); - else { - p += sprintf(p, "%.*d", precision, (*xbt_getpid)()); + if (precision == -1) { + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer), "%d", (*xbt_getpid)()); + check_overflow; + } else { + p += sprintf(p, "%.*d", MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision), (*xbt_getpid)()); + check_overflow; precision = -1; } break; case 'F': /* file name; LOG4J compliant */ - if (precision == -1) - p += sprintf(p,"%s",ev->fileName); - else { - p += sprintf(p,"%.*s",precision, ev->fileName); + if (precision == -1) { + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer),"%s",ev->fileName); + check_overflow; + } else { + p += sprintf(p,"%.*s",MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision), ev->fileName); + check_overflow; precision = -1; } break; case 'l': /* location; LOG4J compliant */ - if (precision == -1) - p += sprintf(p, "%s:%d", ev->fileName, ev->lineNum); - else { - p += snprintf(p, precision, "%s:%d", ev->fileName, ev->lineNum); + if (precision == -1) { + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer), "%s:%d", ev->fileName, ev->lineNum); + check_overflow; + } else { + p += snprintf(p, MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision), "%s:%d", ev->fileName, ev->lineNum); + check_overflow; precision = -1; } break; case 'L': /* line number; LOG4J compliant */ - if (precision == -1) - p += sprintf(p, "%d", ev->lineNum); - else { - p += sprintf(p, "%.*d", precision, ev->lineNum); + if (precision == -1) { + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer), "%d", ev->lineNum); + check_overflow; + } else { + p += sprintf(p, "%.*d", MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision), ev->lineNum); + check_overflow; precision = -1; } break; case 'M': /* method (ie, function) name; LOG4J compliant */ - if (precision == -1) - p += sprintf(p, "%s", ev->functionName); - else { - p += sprintf(p, "%.*s", precision, ev->functionName); + if (precision == -1) { + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer), "%s", ev->functionName); + check_overflow; + } else { + p += sprintf(p, "%.*s", MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision), ev->functionName); + check_overflow; precision = -1; } break; @@ -149,51 +178,62 @@ static void xbt_log_layout_format_doit(xbt_log_layout_t l, e.remote=0; xbt_backtrace_current(&e); if (*q=='B') { - if (precision == -1) - p += sprintf(p,"%s",e.bt_strings[2]+8); - else { - p += sprintf(p,"%.*s",precision, e.bt_strings[2]+8); + if (precision == -1) { + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer),"%s",e.bt_strings[2]+8); + check_overflow; + } else { + p += sprintf(p,"%.*s",MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision), e.bt_strings[2]+8); + check_overflow; precision = -1; } } else { for (i=2; ibuffer),"%s\n",e.bt_strings[i]+8); + check_overflow; + } else { + p += sprintf(p,"%.*s\n",MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision),e.bt_strings[i]+8); + check_overflow; + precision = -1; } } xbt_ex_free(e); } #else - p+=sprintf(p,"(no backtrace on this arch)"); + p+=snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer),"(no backtrace on this arch)"); + check_overflow; #endif break; case 'd': /* date; LOG4J compliant */ - if (precision == -1) - p += sprintf(p,"%f", gras_os_time()); - else { - p += sprintf(p,"%.*f", precision, gras_os_time()); + if (precision == -1) { + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer),"%f", gras_os_time()); + check_overflow; + } else { + p += sprintf(p,"%.*f", MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision), gras_os_time()); + check_overflow; precision = -1; } break; case 'r': /* application age; LOG4J compliant */ - if (precision == -1) - p += sprintf(p,"%f", gras_os_time()-begin_of_time); - else { - p += sprintf(p,"%.*f", precision, gras_os_time()-begin_of_time); + if (precision == -1) { + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer),"%f", gras_os_time()-begin_of_time); + check_overflow; + } else { + p += sprintf(p,"%.*f", MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision), gras_os_time()-begin_of_time); + check_overflow; precision = -1; } break; case 'm': /* user-provided message; LOG4J compliant */ - if (precision == -1) - p += vsprintf(p, msg_fmt, ev->ap); - else { - p += vsnprintf(p, precision, msg_fmt, ev->ap); + if (precision == -1) { + p += vsnprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer), msg_fmt, ev->ap); + check_overflow; + } else { + p += vsnprintf(p, MIN(XBT_LOG_BUFF_SIZE-(p-ev->buffer),precision), msg_fmt, ev->ap); + check_overflow; precision = -1; } break; diff --git a/src/xbt/xbt_log_layout_simple.c b/src/xbt/xbt_log_layout_simple.c index 9185cb2e5d..5363c18388 100644 --- a/src/xbt/xbt_log_layout_simple.c +++ b/src/xbt/xbt_log_layout_simple.c @@ -15,6 +15,13 @@ extern const char *xbt_log_priority_names[7]; +/* only used after the format using: we suppose that the buffer is big enough to display our data */ +#define check_overflow \ + if (p-ev->buffer > XBT_LOG_BUFF_SIZE) { /* buffer overflow */ \ + p=ev->buffer + XBT_LOG_BUFF_SIZE - strlen(" >> OUTPUT TRUNCATED <<\n"); \ + p+=sprintf(p," >> OUTPUT TRUNCATED <<\n"); \ + } + static void xbt_log_layout_simple_doit(xbt_log_layout_t l, xbt_log_event_t ev, const char *fmt) { @@ -31,30 +38,31 @@ static void xbt_log_layout_simple_doit(xbt_log_layout_t l, begin_of_time=gras_os_time(); p = ev->buffer; - p += sprintf(p,"[");; + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer),"["); + /* Display the proc info if available */ if(strlen(xbt_procname())) - p += sprintf(p,"%s:%s:(%d) ", + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer),"%s:%s:(%d) ", gras_os_myname(), xbt_procname(),(*xbt_getpid)()); /* Display the date */ - p += sprintf(p,"%f] ", gras_os_time()-begin_of_time); - + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer),"%f] ", gras_os_time()-begin_of_time); /* Display file position if not INFO*/ if (ev->priority != xbt_log_priority_info) - p += sprintf(p, "%s:%d: ", ev->fileName, ev->lineNum); + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer), "%s:%d: ", ev->fileName, ev->lineNum); /* Display category name */ - p += sprintf(p, "[%s/%s] ", + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer), "[%s/%s] ", ev->cat->name, xbt_log_priority_names[ev->priority] ); /* Display user-provided message */ - p += vsprintf(p, fmt, ev->ap); - + p += vsnprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer), fmt, ev->ap); + check_overflow; + /* End it */ - p += sprintf(p, "\n"); - + p += snprintf(p,XBT_LOG_BUFF_SIZE-(p-ev->buffer), "\n"); + check_overflow; } xbt_log_layout_t xbt_log_layout_simple_new(char *arg) { -- 2.20.1