From f75772231a0e3d0dee046cee23993a4dbc066939 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 22 Sep 2005 16:08:16 +0000 Subject: security hardening of the new "call script" action --- NEWS | 3 +++ stringbuf.c | 19 +++++++++++++++++++ syslogd.c | 44 +++++++++++++++++++++++++++++++++++++++----- 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index 39820b40..82571102 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ Version 1.10.1 (RGer), 2005-09-2? - added the ability to execute a shell script as an action. Thanks to Bjoern Kalkbrenner for providing the code! +- fixed a bug in the MySQL code; due to the bug the automatic one-time + retry after an error did not happen - this lead to error message in + cases where none should be seen (e.g. after a MySQL restart) --------------------------------------------------------------------------- Version 1.10.0 (RGer), 2005-09-20 REMINDER: 1.10 is the first unstable version if the 1.x series! diff --git a/stringbuf.c b/stringbuf.c index c3e0fee0..85763f27 100755 --- a/stringbuf.c +++ b/stringbuf.c @@ -92,14 +92,33 @@ void rsCStrDestruct(rsCStrObj *pThis) rsRetVal rsCStrAppendStr(rsCStrObj *pThis, char* psz) { rsRetVal iRet; + int iOldAllocInc; + int iStrLen; rsCHECKVALIDOBJECT(pThis, OIDrsCStr); assert(psz != NULL); + /* we first check if the to-be-added string is larger than the + * alloc increment. If so, we temporarily increase the alloc + * increment to the length of the string. This will ensure that + * one string copy will be needed at most. As this is a very + * costly operation, it outweights the cost of the strlen() and + * related stuff - at least I think so. + * rgerhards 2005-09-22 + */ + /* We save the current alloc increment in any case, so we can just + * overwrite it below, this is faster than any if-construct. + */ + iOldAllocInc = pThis->iAllocIncrement; + if((iStrLen = strlen(psz)) > pThis->iAllocIncrement) { + pThis->iAllocIncrement = iStrLen; + } + while(*psz) if((iRet = rsCStrAppendChar(pThis, *psz++)) != RS_RET_OK) return iRet; + pThis->iAllocIncrement = iOldAllocInc; /* restore */ return RS_RET_OK; } diff --git a/syslogd.c b/syslogd.c index 5fc87443..6f0b2ebf 100644 --- a/syslogd.c +++ b/syslogd.c @@ -3967,6 +3967,8 @@ void fprintlog(register struct filed *f, int flags) char *psz; /* for shell support */ int esize; /* for shell support */ char *exec; /* for shell support */ + rsCStrObj *pCSCmdLine; /* for shell support: command to execute */ + rsRetVal iRet; #ifdef SYSLOG_INET register int l; time_t fwd_suspend; @@ -4141,17 +4143,49 @@ void fprintlog(register struct filed *f, int flags) if (l > MAXLINE) l = MAXLINE; esize = strlen(f->f_un.f_fname) + strlen(psz) + 4; - exec = (char*) calloc(1, esize * sizeof(char)); - strcpy(exec,f->f_un.f_fname); - strcat(exec," \""); - strcat(exec,psz); - strcat(exec,"\""); + if((pCSCmdLine = rsCStrConstruct()) == NULL) { + /* nothing smart we can do - just keep going... */ + dprintf("memory shortage - can not execute\n"); + break; + } + if((iRet = rsCStrAppendStr(pCSCmdLine, f->f_un.f_fname)) != RS_RET_OK) { + dprintf("error %d during build command line(1)\n", iRet); + break; + } + if((iRet = rsCStrAppendStr(pCSCmdLine, " \"")) != RS_RET_OK) { + dprintf("error %d during build command line(2)\n", iRet); + break; + } + /* now copy the message as parameter but escape dangerous things. + * we probably have not taken care of everything an attacker might + * think of, so execute shell *is* a dangerous command. + * rgerhards 2005-09-22 + */ + while(*psz) { + if(*psz == '"' || *psz == '\\') + if((iRet = rsCStrAppendChar(pCSCmdLine, '\\')) != RS_RET_OK) { + dprintf("error %d during build command line(3)\n", iRet); + break; + } + if((iRet = rsCStrAppendChar(pCSCmdLine, *psz)) != RS_RET_OK) { + dprintf("error %d during build command line(4)\n", iRet); + break; + } + ++psz; + } + if((iRet = rsCStrAppendChar(pCSCmdLine, '"')) != RS_RET_OK) { + dprintf("error %d during build command line(5)\n", iRet); + break; + } + rsCStrFinish(pCSCmdLine); + exec = rsCStrConvSzStrAndDestruct(pCSCmdLine); dprintf("Executing \"%s\"\n",exec); system(exec); /* rgerhards: TODO: need to change this for root jail support! */ free(exec); break; } /* switch */ + if (f->f_type != F_FORW_UNKN) f->f_prevcount = 0; return; -- cgit