fixing MySQL escaping issue

1 files changed, 49 insertions, 18 deletions

diff --git a/syslogd.c b/syslogd.c
index 5d3bea44..594cbc4b 100644
--- a/syslogd.c
+++ b/syslogd.c
@@ -3385,25 +3385,43 @@ void logmsg(pri, pMsg, flags)
  * signatures eventually present - but this is the
  * best thing we can do now (or does anybody 
  * have a better idea?). rgerhards 2004-11-23
+ * added support for "escapeMode" (so doSQLEscape for details).
+ * if mode = 1, then backslashes are changed to slashes.
+ * rgerhards 2005-09-22
  */
-void doSQLEmergencyEscape(register char *p)
+void doSQLEmergencyEscape(register char *p, int escapeMode)
 {
 	while(*p) {
 		if(*p == '\'')
 			*p = '"';
+		else if((escapeMode == 1) && (*p == '\\'))
+			*p = '/';
 		++p;
 	}
 }
 
-
 /* SQL-Escape a string. Single quotes are found and
  * replaced by two of them. A new buffer is allocated
  * for the provided string and the provided buffer is
  * freed. The length is updated. Parameter pbMustBeFreed
  * is set to 1 if a new buffer is allocated. Otherwise,
  * it is left untouched.
+ * --
+ * We just discovered a security issue. MySQL is so
+ * "smart" to not only support the standard SQL mechanism
+ * for escaping quotes, but to also provide its own (using
+ * c-type syntax with backslashes). As such, it is actually
+ * possible to do sql injection via rsyslogd. The cure is now
+ * to escape backslashes, too. As we have found on the web, some
+ * other databases seem to be similar "smart" (why do we have standards
+ * at all if they are violated without any need???). Even better, MySQL's
+ * smartness depends on config settings. So we add a new option to this
+ * function that allows the caller to select if they want to standard or
+ * "smart" encoding ;)
+ * new parameter escapeMode is 0 - standard sql, 1 - "smart" engines
+ * 2005-09-22 rgerhards
  */
-void doSQLEscape(char **pp, size_t *pLen, unsigned short *pbMustBeFreed)
+void doSQLEscape(char **pp, size_t *pLen, unsigned short *pbMustBeFreed, int escapeMode)
 {
 	char *p;
 	int iLen;
@@ -3416,8 +3434,12 @@ void doSQLEscape(char **pp, size_t *pLen, unsigned short *pbMustBeFreed)
 	assert(pbMustBeFreed != NULL);
 
 	/* first check if we need to do anything at all... */
-	for(p = *pp ; *p && *p != '\'' ; ++p)
-		;
+	if(escapeMode == 0)
+		for(p = *pp ; *p && *p != '\'' ; ++p)
+			;
+	else
+		for(p = *pp ; *p && *p != '\'' && *p != '\\' ; ++p)
+			;
 	/* when we get out of the loop, we are either at the
 	 * string terminator or the first \'. */
 	if(*p == '\0')
@@ -3427,32 +3449,38 @@ void doSQLEscape(char **pp, size_t *pLen, unsigned short *pbMustBeFreed)
 	iLen = *pLen;
 	if((pStrB = sbStrBConstruct()) == NULL) {
 		/* oops - no mem ... Do emergency... */
-		doSQLEmergencyEscape(p);
+		doSQLEmergencyEscape(p, escapeMode);
 		return;
 	}
 	
 	while(*p) {
 		if(*p == '\'') {
-			if(sbStrBAppendChar(pStrB, '\'') != SR_RET_OK) {
-				doSQLEmergencyEscape(*pp);
-				if((pszGenerated = sbStrBFinish(pStrB))
-					!= NULL)
+			if(sbStrBAppendChar(pStrB, (escapeMode == 0) ? '\'' : '\\') != SR_RET_OK) {
+				doSQLEmergencyEscape(*pp, escapeMode);
+				if((pszGenerated = sbStrBFinish(pStrB)) != NULL) 
 					free(pszGenerated);
-					return;
+				return;
+			}
 			iLen++;	/* reflect the extra character */
+		} else if((escapeMode == 1) && (*p == '\\')) {
+			if(sbStrBAppendChar(pStrB, '\\') != SR_RET_OK) {
+				doSQLEmergencyEscape(*pp, escapeMode);
+				if((pszGenerated = sbStrBFinish(pStrB)) != NULL) 
+					free(pszGenerated);
+				return;
 			}
+			iLen++;	/* reflect the extra character */
 		}
 		if(sbStrBAppendChar(pStrB, *p) != SR_RET_OK) {
-			doSQLEmergencyEscape(*pp);
-			if((pszGenerated = sbStrBFinish(pStrB))
-				!= NULL)
+			doSQLEmergencyEscape(*pp, escapeMode);
+			if((pszGenerated = sbStrBFinish(pStrB)) != NULL)
 				free(pszGenerated);
-				return;
+			return;
 		}
 		++p;
 	}
 	if((pszGenerated = sbStrBFinish(pStrB)) == NULL) {
-		doSQLEmergencyEscape(*pp);
+		doSQLEmergencyEscape(*pp, escapeMode);
 		return;
 	}
 
@@ -3575,9 +3603,12 @@ void  iovCreate(struct filed *f)
 			/* we now need to check if we should use SQL option. In this case,
 			 * we must go over the generated string and escape '\'' characters.
 			 */
-			if(f->f_pTpl->optFormatForSQL)
+			if(f->f_pTpl->optFormatForSQL == 1)
+				doSQLEscape((char**)&v->iov_base, &v->iov_len,
+					    f->f_bMustBeFreed + iIOVused, 1);
+			else if(f->f_pTpl->optFormatForSQL == 2)
 				doSQLEscape((char**)&v->iov_base, &v->iov_len,
-					    f->f_bMustBeFreed + iIOVused);
+					    f->f_bMustBeFreed + iIOVused, 0);
 			++v;
 			++iIOVused;
 		}