diff options
| -rw-r--r-- | ChangeLog | 6 | ||||
| -rw-r--r-- | runtime/datetime.c | 17 | ||||
| -rw-r--r-- | runtime/rsyslog.h | 2 | ||||
| -rw-r--r-- | tests/testsuites/weird.parse1 | 34 | ||||
| -rw-r--r-- | tools/syslogd.c | 16 |
5 files changed, 64 insertions, 11 deletions
@@ -1,3 +1,9 @@ +- improved testbench to contain samples for totally malformed messages + which miss parts of the message content +- bugfix: some malformed messages could lead to a missing LF inside files + or some other missing parts of the template content. +- bugfix: if a message ended immediately with a hostname, the hostname + was mistakenly interpreted as TAG, and localhost be used as hostname - bugfix: message without MSG part could case a segfault [backported from v5 commit 98d1ed504ec001728955a5bcd7916f64cd85f39f] This actually was a "recent" regression, but I did not realize that it diff --git a/runtime/datetime.c b/runtime/datetime.c index 6160bd7c..7b0d8d11 100644 --- a/runtime/datetime.c +++ b/runtime/datetime.c @@ -291,11 +291,11 @@ ParseTIMESTAMP3339(struct syslogTime *pTime, uchar** ppszTS, int *pLenStr) } /* OK, we actually have a 3339 timestamp, so let's indicated this */ - if(lenStr > 0 && *pszTS == ' ') { + if(lenStr > 0) { + if(*pszTS != ' ') /* if it is not a space, it can not be a "good" time - 2010-02-22 rgerhards */ + ABORT_FINALIZE(RS_RET_INVLD_TIME); + ++pszTS; /* just skip past it */ --lenStr; - ++pszTS; - } else { - ABORT_FINALIZE(RS_RET_INVLD_TIME); } /* we had success, so update parse pointer and caller-provided timestamp */ @@ -510,6 +510,7 @@ ParseTIMESTAMP3164(struct syslogTime *pTime, uchar** ppszTS, int *pLenStr) if(lenStr == 0 || *pszTS++ != ' ') ABORT_FINALIZE(RS_RET_INVLD_TIME); + --lenStr; /* we accept a slightly malformed timestamp when receiving. This is * we accept one-digit days @@ -565,7 +566,13 @@ ParseTIMESTAMP3164(struct syslogTime *pTime, uchar** ppszTS, int *pLenStr) * invalid format, it occurs frequently enough (e.g. with Cisco devices) * to permit it as a valid case. -- rgerhards, 2008-09-12 */ - if(lenStr == 0 || *pszTS++ == ':') { + if(lenStr > 0 && *pszTS == ':') { + ++pszTS; /* just skip past it */ + --lenStr; + } + if(lenStr > 0) { + if(*pszTS != ' ') /* if it is not a space, it can not be a "good" time - 2010-02-22 rgerhards */ + ABORT_FINALIZE(RS_RET_INVLD_TIME); ++pszTS; /* just skip past it */ --lenStr; } diff --git a/runtime/rsyslog.h b/runtime/rsyslog.h index 27bea6bc..0f489a7f 100644 --- a/runtime/rsyslog.h +++ b/runtime/rsyslog.h @@ -35,7 +35,7 @@ * value to the fixed size of the message object. */ #define CONF_TAG_MAXSIZE 512 /* a value that is deemed far too large for any valid TAG */ -#define CONF_TAG_HOSTNAME 512 /* a value that is deemed far too large for any valid HOSTNAME */ +#define CONF_HOSTNAME_MAXSIZE 512 /* a value that is deemed far too large for any valid HOSTNAME */ #define CONF_RAWMSG_BUFSIZE 101 #define CONF_TAG_BUFSIZE 32 #define CONF_HOSTNAME_BUFSIZE 32 diff --git a/tests/testsuites/weird.parse1 b/tests/testsuites/weird.parse1 index bc898fd4..e8b90c74 100644 --- a/tests/testsuites/weird.parse1 +++ b/tests/testsuites/weird.parse1 @@ -2,4 +2,36 @@ # some other deliberately generated. The main point is that they # should not cause an abort... <14>Aug 30 23:00:05 X4711 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA -14,user,info,Aug 30 23:00:05,X4711,AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA,AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA +14,user,info,Aug 30 23:00:05,X4711,AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA,AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA, +# important: the following line has a SP at the end of the line! +<14>Aug 30 23:00:05 X4711 +14,user,info,Aug 30 23:00:05,X4711,,, +# and this one NOT +<14>Aug 30 23:00:05 X4711 +14,user,info,Aug 30 23:00:05,X4711,,, +# there is a SP at the end of the line +<14>Aug 30 23:00:05 +14,user,info,Aug 30 23:00:05,localhost,,, +# and here is no SP at the end of the line +<14>Aug 30 23:00:05 +14,user,info,Aug 30 23:00:05,localhost,,, +# unfortunately, I can not test missing dates with this test suite, because +# we would have the current date in the response, which we can not check against +# +# and now the same tests with RFC3339 data - this can make a difference +# as a different date parser is involved. +# +<14>2010-08-30T23:00:05Z X4711 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA +14,user,info,Aug 30 23:00:05,X4711,AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA,AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA, +# important: the following line has a SP at the end of the line! +<14>2010-08-30T23:00:05Z X4711 +14,user,info,Aug 30 23:00:05,X4711,,, +# and this one NOT +<14>2010-08-30T23:00:05Z X4711 +14,user,info,Aug 30 23:00:05,X4711,,, +# there is a SP at the end of the line +<14>2010-08-30T23:00:05Z +14,user,info,Aug 30 23:00:05,localhost,,, +# and here is no SP at the end of the line +<14>2010-08-30T23:00:05Z +14,user,info,Aug 30 23:00:05,localhost,,, diff --git a/tools/syslogd.c b/tools/syslogd.c index 3e6d51d3..caab1691 100644 --- a/tools/syslogd.c +++ b/tools/syslogd.c @@ -1192,12 +1192,12 @@ int parseLegacySyslogMsg(msg_t *pMsg, int flags) int bTAGCharDetected; int i; /* general index for parsing */ uchar bufParseTAG[CONF_TAG_MAXSIZE]; - uchar bufParseHOSTNAME[CONF_TAG_HOSTNAME]; + uchar bufParseHOSTNAME[CONF_HOSTNAME_MAXSIZE]; BEGINfunc assert(pMsg != NULL); assert(pMsg->pszRawMsg != NULL); - lenMsg = pMsg->iLenRawMsg - (pMsg->offAfterPRI + 1); + lenMsg = pMsg->iLenRawMsg - pMsg->offAfterPRI; /* note: offAfterPRI is already the number of PRI chars (do not add one!) */ p2parse = pMsg->pszRawMsg + pMsg->offAfterPRI; /* point to start of text, after PRI */ /* Check to see if msg contains a timestamp. We start by assuming @@ -1254,12 +1254,20 @@ int parseLegacySyslogMsg(msg_t *pMsg, int flags) if(lenMsg > 0 && flags & PARSE_HOSTNAME) { i = 0; while(i < lenMsg && (isalnum(p2parse[i]) || p2parse[i] == '.' || p2parse[i] == '.' - || p2parse[i] == '_' || p2parse[i] == '-') && i < CONF_TAG_MAXSIZE) { + || p2parse[i] == '_' || p2parse[i] == '-') && i < (CONF_HOSTNAME_MAXSIZE - 1)) { bufParseHOSTNAME[i] = p2parse[i]; ++i; } - if(i > 0 && p2parse[i] == ' ' && isalnum(p2parse[i-1])) { + if(i == lenMsg) { + /* we have a message that is empty immediately after the hostname, + * but the hostname thus is valid! -- rgerhards, 2010-02-22 + */ + p2parse += i; + lenMsg -= i; + bufParseHOSTNAME[i] = '\0'; + MsgSetHOSTNAME(pMsg, bufParseHOSTNAME, i); + } else if(i > 0 && p2parse[i] == ' ' && isalnum(p2parse[i-1])) { /* we got a hostname! */ p2parse += i + 1; /* "eat" it (including SP delimiter) */ lenMsg -= i + 1; |