CVE-2011-1489: Missing Release of Resource after Effective Lifetime (CWE-772)
value: A memory leak in rsyslog before 5.7.6 was found in the way deamon processed log messages were logged when multiple rulesets were used and some output batches contained messages belonging to more than one ruleset. A local attacker could cause denial of the rsyslogd daemon service via a log message belonging to more than one ruleset.
CWE description: The product does not release a resource after its effective lifetime has ended, i.e., after the resource is no longer needed.
CVSS v3 base score: 5.5
Published: 2019-11-14T02:15Z
Repository: https://github.com/rsyslog/rsyslog
Language: Other
Files changed: ChangeLog, runtime/batch.h, runtime/ruleset.c
Commit date: 2011-02-25 14:14:17 +0100
Commit message: bugfix: fixed a memory leak and potential abort condition

this could happen if multiple rulesets were used and some output batches
contained messages belonging to more than one ruleset.
fixes: http://bugzilla.adiscon.com/show_bug.cgi?id=226
fixes: http://bugzilla.adiscon.com/show_bug.cgi?id=218

Diff: diff --git a/ChangeLog b/ChangeLog
index 5596dfbe..c4cb28b4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,7 +1,12 @@
 ---------------------------------------------------------------------------
 Version 5.7.6  [V5-BETA] (rgerhards), 2011-02-??
+- bugfix: fixed a memory leak and potential abort condition
+  this could happen if multiple rulesets were used and some output batches
+  contained messages belonging to more than one ruleset.
+  fixes: http://bugzilla.adiscon.com/show_bug.cgi?id=226
+  fixes: http://bugzilla.adiscon.com/show_bug.cgi?id=218
 - bugfix: memory leak when $RepeatedMsgReduction on was used
   bug tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=225
 ---------------------------------------------------------------------------
 Version 5.7.5  [V5-BETA] (rgerhards), 2011-02-23
 - enhance: imfile did not yet support multiple rulesets, now added
diff --git a/runtime/batch.h b/runtime/batch.h
index d0504f2b..944889bd 100644
--- a/runtime/batch.h
+++ b/runtime/batch.h
@@ -134,15 +134,20 @@ batchIsValidElem(batch_t *pBatch, int i) {
 
 
 /* copy one batch element to another.
  * This creates a complete duplicate in those cases where
  * it is needed. Use duplication only when absolutely necessary!
+ * Note that all working fields are reset to zeros. If that were 
+ * not done, we would have potential problems with invalid
+ * or double pointer frees.
  * rgerhards, 2010-06-10
  */
 static inline void
 batchCopyElem(batch_obj_t *pDest, batch_obj_t *pSrc) {
-	memcpy(pDest, pSrc, sizeof(batch_obj_t));
+	memset(pDest, 0, sizeof(batch_obj_t));
+	pDest->pUsrp = pSrc->pUsrp;
+	pDest->state = pSrc->state;
 }
 
 
 /* free members of a batch "object". Note that we can not do the usual
  * destruction as the object typically is allocated on the stack and so the
@@ -169,10 +174,11 @@ batchFree(batch_t *pBatch) {
  * provided. -- rgerhards, 2010-06-15
  */
 static inline rsRetVal
 batchInit(batch_t *pBatch, int maxElem) {
 	DEFiRet;
+	pBatch->iDoneUpTo = 0;
 	pBatch->maxElem = maxElem;
 	CHKmalloc(pBatch->pElem = calloc((size_t)maxElem, sizeof(batch_obj_t)));
 	// TODO: replace calloc by inidividual writes?
 finalize_it:
 	RETiRet;
diff --git a/runtime/ruleset.c b/runtime/ruleset.c
index 0584e8d6..c9c64a38 100644
--- a/runtime/ruleset.c
+++ b/runtime/ruleset.c
@@ -169,39 +169,44 @@ processBatchMultiRuleset(batch_t *pBatch)
 	ruleset_t *currRuleset;
 	batch_t snglRuleBatch;
 	int i;
 	int iStart;	/* start index of partial batch */
 	int iNew;	/* index for new (temporary) batch */
+	int bHaveUnprocessed;	/* do we (still) have unprocessed entries? (loop term predicate) */
 	DEFiRet;
 
-	CHKiRet(batchInit(&snglRuleBatch, pBatch->nElem));
-	snglRuleBatch.pbShutdownImmediate = pBatch->pbShutdownImmediate;
-
-	while(1) { /* loop broken inside */
+	do {
+		bHaveUnprocessed = 0;
 		/* search for first unprocessed element */
 		for(iStart = 0 ; iStart < pBatch->nElem && pBatch->pElem[iStart].state == BATCH_STATE_DISC ; ++iStart)
 			/* just search, no action */;
-
 		if(iStart == pBatch->nElem)
-			FINALIZE; /* everything processed */
+			break; /* everything processed */
 
 		/* prepare temporary batch */
+		CHKiRet(batchInit(&snglRuleBatch, pBatch->nElem));
+		snglRuleBatch.pbShutdownImmediate = pBatch->pbShutdownImmediate;
 		currRuleset = batchElemGetRuleset(pBatch, iStart);
 		iNew = 0;
 		for(i = iStart ; i < pBatch->nElem ; ++i) {
 			if(batchElemGetRuleset(pBatch, i) == currRuleset) {
-				batchCopyElem(&(snglRuleBatch.pElem[iNew++]), &(pBatch->pElem[i]));
+				/* for performance reasons, we copy only those members that we actually need */
+				snglRuleBatch.pElem[iNew].pUsrp = pBatch->pElem[i].pUsrp;
+				snglRuleBatch.pElem[iNew].state = pBatch->pElem[i].state;
+				++iNew;
 				/* We indicate the element also as done, so it will not be processed again */
 				pBatch->pElem[i].state = BATCH_STATE_DISC;
+			} else {
+				bHaveUnprocessed = 1;
 			}
 		}
 		snglRuleBatch.nElem = iNew; /* was left just right by the for loop */
 		batchSetSingleRuleset(&snglRuleBatch, 1);
 		/* process temp batch */
 		processBatch(&snglRuleBatch);
-	}
-	batchFree(&snglRuleBatch);
+		batchFree(&snglRuleBatch);
+	} while(bHaveUnprocessed == 1);
 
 finalize_it:
 	RETiRet;
 }
 

Vulnerable code: 	memcpy(pDest, pSrc, sizeof(batch_obj_t));
	CHKiRet(batchInit(&snglRuleBatch, pBatch->nElem));
	snglRuleBatch.pbShutdownImmediate = pBatch->pbShutdownImmediate;

	while(1) { /* loop broken inside */

			FINALIZE; /* everything processed */
				batchCopyElem(&(snglRuleBatch.pElem[iNew++]), &(pBatch->pElem[i]));
	}
	batchFree(&snglRuleBatch);
Fixed code: - bugfix: fixed a memory leak and potential abort condition
  this could happen if multiple rulesets were used and some output batches
  contained messages belonging to more than one ruleset.
  fixes: http://bugzilla.adiscon.com/show_bug.cgi?id=226
  fixes: http://bugzilla.adiscon.com/show_bug.cgi?id=218
 * Note that all working fields are reset to zeros. If that were 
 * not done, we would have potential problems with invalid
 * or double pointer frees.
	memset(pDest, 0, sizeof(batch_obj_t));
	pDest->pUsrp = pSrc->pUsrp;
	pDest->state = pSrc->state;
	pBatch->iDoneUpTo = 0;
	int bHaveUnprocessed;	/* do we (still) have unprocessed entries? (loop term predicate) */
	do {
		bHaveUnprocessed = 0;
			break; /* everything processed */
		CHKiRet(batchInit(&snglRuleBatch, pBatch->nElem));
		snglRuleBatch.pbShutdownImmediate = pBatch->pbShutdownImmediate;
				/* for performance reasons, we copy only those members that we actually need */
				snglRuleBatch.pElem[iNew].pUsrp = pBatch->pElem[i].pUsrp;
				snglRuleBatch.pElem[iNew].state = pBatch->pElem[i].state;
				++iNew;
			} else {
				bHaveUnprocessed = 1;
		batchFree(&snglRuleBatch);
	} while(bHaveUnprocessed == 1);