From 7090e62564bcb7bff7efc89bd2a42b1d32d9e475 Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan.bosch@open-xchange.com>
Date: Wed, 2 Jun 2021 23:19:46 +0200
Subject: [PATCH] lib-sieve: sieve-result - Never allow side-effect definition
 to be NULL.

This had no real purpose and could cause NULL pointer dereference.

Found by Clang scan-build.
---
 src/lib-sieve/sieve-result.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/src/lib-sieve/sieve-result.c b/src/lib-sieve/sieve-result.c
index 3aa360736..90a9b75f0 100644
--- a/src/lib-sieve/sieve-result.c
+++ b/src/lib-sieve/sieve-result.c
@@ -318,7 +318,8 @@ sieve_result_side_effects_merge(const struct sieve_runtime_env *renv,
 		struct sieve_side_effect *seffect = &rsef->seffect;
 		bool found = FALSE;
 
-		if (seffect->def != NULL && seffect->def->merge != NULL) {
+		i_assert(seffect->def != NULL);
+		if (seffect->def->merge != NULL) {
 			/* Try to find it among the new */
 			nrsef = (new_seffects != NULL ?
 				 new_seffects->first_effect : NULL);
@@ -352,7 +353,8 @@ sieve_result_side_effects_merge(const struct sieve_runtime_env *renv,
 		struct sieve_side_effect *nseffect = &nrsef->seffect;
 		bool found = FALSE;
 
-		if (nseffect->def != NULL && nseffect->def->merge != NULL) {
+		i_assert(nseffect->def != NULL);
+		if (nseffect->def->merge != NULL) {
 			/* Try to find it among the exising */
 			rsef = (old_seffects != NULL ?
 				old_seffects->first_effect : NULL);
@@ -747,13 +749,13 @@ sieve_result_print_side_effects(struct sieve_result_print_env *rpenv,
 	/* Print side effects */
 	rsef = (slist != NULL ? slist->first_effect : NULL);
 	while (rsef != NULL) {
-		if (rsef->seffect.def != NULL) {
-			const struct sieve_side_effect *sef = &rsef->seffect;
+		const struct sieve_side_effect *sef = &rsef->seffect;
 
-			if (sef->def->print != NULL) {
-				sef->def->print(sef, action, rpenv,
-						implicit_keep);
-			}
+		i_assert(sef->def != NULL);
+
+		if (sef->def->print != NULL) {
+			sef->def->print(sef, action, rpenv,
+					implicit_keep);
 		}
 		rsef = rsef->next;
 	}
@@ -953,8 +955,7 @@ sieve_result_side_effect_pre_execute(struct sieve_result_execution *rexec,
 	struct sieve_result_side_effect *rsef = seexec->seffect;
 	struct sieve_side_effect *sef = &rsef->seffect;
 
-	if (sef->def == NULL)
-		return SIEVE_EXEC_OK;
+	i_assert(sef->def != NULL);
 	if (sef->def->pre_execute == NULL)
 		return SIEVE_EXEC_OK;
 
@@ -971,8 +972,7 @@ sieve_result_side_effect_post_execute(
 	struct sieve_result_side_effect *rsef = seexec->seffect;
 	struct sieve_side_effect *sef = &rsef->seffect;
 
-	if (sef->def == NULL)
-		return SIEVE_EXEC_OK;
+	i_assert(sef->def != NULL);
 	if (sef->def->post_execute == NULL)
 		return SIEVE_EXEC_OK;
 
@@ -990,8 +990,7 @@ sieve_result_side_effect_post_commit(struct sieve_result_execution *rexec,
 	struct sieve_result_side_effect *rsef = seexec->seffect;
 	struct sieve_side_effect *sef = &rsef->seffect;
 
-	if (sef->def == NULL)
-		return;
+	i_assert(sef->def != NULL);
 	if (sef->def->post_commit == NULL)
 		return;
 
@@ -1008,8 +1007,7 @@ sieve_result_side_effect_rollback(struct sieve_result_execution *rexec,
 	struct sieve_result_side_effect *rsef = seexec->seffect;
 	struct sieve_side_effect *sef = &rsef->seffect;
 
-	if (sef->def == NULL)
-		return;
+	i_assert(sef->def != NULL);
 	if (sef->def->rollback == NULL)
 		return;
 
@@ -2102,6 +2100,9 @@ void sieve_side_effects_list_add(struct sieve_side_effects_list *list,
 		const struct sieve_side_effect_def *ref_def = reffect->seffect.def;
 		const struct sieve_side_effect_def *sef_def = seffect->def;
 
+		i_assert(ref_def != NULL);
+		i_assert(sef_def != NULL);
+
 		if (sef_def == ref_def) {
 			/* already listed */
 			i_assert(reffect_pos == NULL);
-- 
GitLab