CVE-2011-1488: 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 are logged when $RepeatedMsgReduction was enabled. A local attacker could use this flaw to cause a denial of the rsyslogd daemon service by crashing the service via a sequence of repeated log messages sent within short periods of time.
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);