security hardening of the new "call script" action

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;