From ec56b763b83677d1e9cd02a7ae610caf62e902bb Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 8 Oct 2009 16:36:17 +0200 Subject: bugfix in debug system and more instrumentation to find an issue bugfix: debug string larger than 1K were improperly displayed. Max size is now 32K, and if a string is even longer it is meaningful truncated. --- ChangeLog | 4 ++++ configure.ac | 2 +- plugins/imudp/imudp.c | 3 ++- runtime/debug.c | 12 +++++++++++- runtime/parser.c | 16 ++++++++-------- template.c | 6 +++++- 6 files changed, 31 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8759c846..1c81f3aa 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,8 @@ --------------------------------------------------------------------------- +Version 5.3.2 [DEVEL] (rgerhards), 2009-10-?? +- bugfix: debug string larger than 1K were improperly displayed. Max size + is now 32K, and if a string is even longer it is meaningful truncated. +--------------------------------------------------------------------------- Version 5.3.1 [DEVEL] (rgerhards), 2009-10-05 - added $AbortOnUncleanConfig directive - permits to prevent startup when there are problems with the configuration file. See it's doc for diff --git a/configure.ac b/configure.ac index 07fd0dac..e3f60b5c 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ(2.61) -AC_INIT([rsyslog],[5.3.1],[rsyslog@lists.adiscon.com]) +AC_INIT([rsyslog],[5.3.2],[rsyslog@lists.adiscon.com]) AM_INIT_AUTOMAKE m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])]) diff --git a/plugins/imudp/imudp.c b/plugins/imudp/imudp.c index 735042a4..3fabf1a2 100644 --- a/plugins/imudp/imudp.c +++ b/plugins/imudp/imudp.c @@ -244,7 +244,8 @@ processSocket(int fd, struct sockaddr_storage *frominetPrev, int *pbIsPermitted, } } - DBGPRINTF("recv(%d,%d)/%s,acl:%d,msg:%.80s\n", fd, (int) lenRcvBuf, fromHost, *pbIsPermitted, pRcvBuf); + //DBGPRINTF("recv(%d,%d)/%s,acl:%d,msg:%.80s\n", fd, (int) lenRcvBuf, fromHost, *pbIsPermitted, pRcvBuf); + DBGPRINTF("recv(%d,%d)/%s,acl:%d,msg:%s\n", fd, (int) lenRcvBuf, fromHost, *pbIsPermitted, pRcvBuf); if(*pbIsPermitted) { if((iTimeRequery == 0) || (iNbrTimeUsed++ % iTimeRequery) == 0) { diff --git a/runtime/debug.c b/runtime/debug.c index fb751efb..7c938008 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -960,7 +960,7 @@ void dbgprintf(char *fmt, ...) { va_list ap; - char pszWriteBuf[1024]; + char pszWriteBuf[32*1024]; size_t lenWriteBuf; if(!(Debug && debugging_on)) @@ -969,6 +969,16 @@ dbgprintf(char *fmt, ...) va_start(ap, fmt); lenWriteBuf = vsnprintf(pszWriteBuf, sizeof(pszWriteBuf), fmt, ap); va_end(ap); + + if(lenWriteBuf >= sizeof(pszWriteBuf)) { + /* if we need to truncate, do it in a somewhat useful way... */ + pszWriteBuf[sizeof(pszWriteBuf) - 5] = '!'; + pszWriteBuf[sizeof(pszWriteBuf) - 4] = '.'; + pszWriteBuf[sizeof(pszWriteBuf) - 3] = '.'; + pszWriteBuf[sizeof(pszWriteBuf) - 2] = '.'; + pszWriteBuf[sizeof(pszWriteBuf) - 1] = '\n'; + lenWriteBuf = sizeof(pszWriteBuf); + } dbgprint(NULL, pszWriteBuf, lenWriteBuf); } diff --git a/runtime/parser.c b/runtime/parser.c index 466066e7..3c90c447 100644 --- a/runtime/parser.c +++ b/runtime/parser.c @@ -231,14 +231,14 @@ sanitizeMessage(msg_t *pMsg) * can not handle it! -- rgerhards, 2009-08-26 */ if(pszMsg[iSrc] == '\0' || bEscapeCCOnRcv) { - /* we are configured to escape control characters. Please note - * that this most probably break non-western character sets like - * Japanese, Korean or Chinese. rgerhards, 2007-07-17 - */ - pDst[iDst++] = cCCEscapeChar; - pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0300) >> 6); - pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0070) >> 3); - pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0007)); + /* we are configured to escape control characters. Please note + * that this most probably break non-western character sets like + * Japanese, Korean or Chinese. rgerhards, 2007-07-17 + */ + pDst[iDst++] = cCCEscapeChar; + pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0300) >> 6); + pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0070) >> 3); + pDst[iDst++] = '0' + ((pszMsg[iSrc] & 0007)); } } else { pDst[iDst++] = pszMsg[iSrc]; diff --git a/template.c b/template.c index f3a8e057..1e0c9613 100644 --- a/template.c +++ b/template.c @@ -86,6 +86,7 @@ rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar **ppBuf, size_t * unsigned short bMustBeFreed; uchar *pVal; size_t iLenVal; +int propid = -1; assert(pTpl != NULL); assert(pMsg != NULL); @@ -101,10 +102,12 @@ rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar **ppBuf, size_t * iBuf = 0; while(pTpe != NULL) { if(pTpe->eEntryType == CONSTANT) { +propid = -1; pVal = (uchar*) pTpe->data.constant.pConstant; iLenVal = pTpe->data.constant.iLenConstant; bMustBeFreed = 0; } else if(pTpe->eEntryType == FIELD) { +propid = pTpe->data.field.propid; pVal = (uchar*) MsgGetProp(pMsg, pTpe, pTpe->data.field.propid, &iLenVal, &bMustBeFreed); /* we now need to check if we should use SQL option. In this case, * we must go over the generated string and escape '\'' characters. @@ -118,7 +121,8 @@ rsRetVal tplToString(struct template *pTpl, msg_t *pMsg, uchar **ppBuf, size_t * doSQLEscape(&pVal, &iLenVal, &bMustBeFreed, 0); } /* got source, now copy over */ - if(iBuf + iLenVal + 1 >= *pLenBuf) /* we reserve one char for the final \0! */ +dbgprintf("copying prop id %3d (entry type %d) of length %d ('%s')\n", propid, pTpe->eEntryType, (int) iLenVal, pVal); + if(iBuf + iLenVal >= *pLenBuf) /* we reserve one char for the final \0! */ CHKiRet(ExtendBuf(ppBuf, pLenBuf, iBuf + iLenVal + 1)); if(iLenVal > 0) { /* may be zero depending on property */ -- cgit From 3ed4b2cd3ebaf6f4c377ba2e03ef52c2e8a985b6 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 9 Oct 2009 14:48:25 +0200 Subject: bugfix: potential segfault on messages with empty MSG part. This was a recently introduced regression. --- ChangeLog | 2 ++ runtime/msg.c | 17 ++++++++++++++--- tools/syslogd.c | 6 ++---- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1c81f3aa..a9c1ad07 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,7 @@ --------------------------------------------------------------------------- Version 5.3.2 [DEVEL] (rgerhards), 2009-10-?? +- bugfix: potential segfault on messages with empty MSG part. This was a + recently introduced regression. - bugfix: debug string larger than 1K were improperly displayed. Max size is now 32K, and if a string is even longer it is meaningful truncated. --------------------------------------------------------------------------- diff --git a/runtime/msg.c b/runtime/msg.c index 5a33837f..2c1af27e 100644 --- a/runtime/msg.c +++ b/runtime/msg.c @@ -1177,7 +1177,7 @@ uchar *getMSG(msg_t *pM) if(pM == NULL) ret = UCHAR_CONSTANT(""); else { - if(pM->offMSG == -1) + if(pM->iLenMSG == 0) ret = UCHAR_CONSTANT(""); else ret = pM->pszRawMsg + pM->offMSG; @@ -1953,12 +1953,22 @@ void MsgSetHOSTNAME(msg_t *pThis, uchar* pszHOSTNAME, int lenHOSTNAME) /* set the offset of the MSG part into the raw msg buffer + * Note that the offset may be higher than the length of the raw message + * (exactly by one). This can happen if we have a message that does not + * contain any MSG part. */ void MsgSetMSGoffs(msg_t *pMsg, short offs) { +BEGINfunc ISOBJ_TYPE_assert(pMsg, msg); - pMsg->iLenMSG = pMsg->iLenRawMsg - offs; pMsg->offMSG = offs; + if(offs > pMsg->iLenRawMsg) { + assert(offs - 1 == pMsg->iLenRawMsg); + pMsg->iLenMSG = 0; + } else { + pMsg->iLenMSG = pMsg->iLenRawMsg - offs; + } +ENDfunc } @@ -1992,7 +2002,8 @@ rsRetVal MsgReplaceMSG(msg_t *pThis, uchar* pszMSG, int lenMSG) pThis->pszRawMsg = bufNew; } - memcpy(pThis->pszRawMsg + pThis->offMSG, pszMSG, lenMSG); + if(lenMSG > 0) + memcpy(pThis->pszRawMsg + pThis->offMSG, pszMSG, lenMSG); pThis->pszRawMsg[lenNew] = '\0'; /* this also works with truncation! */ pThis->iLenRawMsg = lenNew; pThis->iLenMSG = lenMSG; diff --git a/tools/syslogd.c b/tools/syslogd.c index 0f4f8a23..3dc2d230 100644 --- a/tools/syslogd.c +++ b/tools/syslogd.c @@ -1205,8 +1205,6 @@ int parseLegacySyslogMsg(msg_t *pMsg, int flags) assert(pMsg != NULL); assert(pMsg->pszRawMsg != NULL); lenMsg = pMsg->iLenRawMsg - (pMsg->offAfterPRI + 1); -RUNLOG_VAR("%d", pMsg->offAfterPRI); -RUNLOG_VAR("%d", lenMsg); p2parse = pMsg->pszRawMsg + pMsg->offAfterPRI; /* point to start of text, after PRI */ /* Check to see if msg contains a timestamp. We start by assuming @@ -1262,16 +1260,16 @@ RUNLOG_VAR("%d", lenMsg); bTAGCharDetected = 0; if(lenMsg > 0 && flags & PARSE_HOSTNAME) { i = 0; - while(lenMsg > 0 && (isalnum(p2parse[i]) || p2parse[i] == '.' || p2parse[i] == '.' + while(i < lenMsg && (isalnum(p2parse[i]) || p2parse[i] == '.' || p2parse[i] == '.' || p2parse[i] == '_' || p2parse[i] == '-') && i < CONF_TAG_MAXSIZE) { bufParseHOSTNAME[i] = p2parse[i]; ++i; - --lenMsg; } if(i > 0 && p2parse[i] == ' ' && isalnum(p2parse[i-1])) { /* we got a hostname! */ p2parse += i + 1; /* "eat" it (including SP delimiter) */ + lenMsg -= i + 1; bufParseHOSTNAME[i] = '\0'; MsgSetHOSTNAME(pMsg, bufParseHOSTNAME, i); } -- cgit