From 51030fcea5b4dec8789d0f2cc61441086b11d33c Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan@rename-it.nl>
Date: Tue, 26 Aug 2008 22:11:29 +0200
Subject: [PATCH] Implemented policy limit on the maximum number of redirect
 actions in a result.

---
 TODO                                          |  1 -
 src/lib-sieve/cmd-discard.c                   |  2 +-
 src/lib-sieve/cmd-redirect.c                  |  3 ++-
 src/lib-sieve/ext-reject.c                    |  2 +-
 src/lib-sieve/plugins/vacation/cmd-vacation.c |  2 +-
 src/lib-sieve/sieve-actions.c                 |  2 +-
 src/lib-sieve/sieve-result.c                  | 18 ++++++++++----
 src/lib-sieve/sieve-result.h                  |  2 +-
 tests/execute/errors.svtest                   | 22 +++++++++++++++--
 tests/execute/errors/actions-limit.sieve      | 24 +++++++++----------
 tests/execute/errors/redirect-limit.sieve     |  5 ++++
 11 files changed, 58 insertions(+), 25 deletions(-)
 create mode 100644 tests/execute/errors/redirect-limit.sieve

diff --git a/TODO b/TODO
index 40eea335f..29a00685c 100644
--- a/TODO
+++ b/TODO
@@ -15,7 +15,6 @@ Next (in order of descending priority/precedence):
 	  the string argument; implementations MUST convert the string to [RFC2047] 
 	  encoded words if and only if non-ASCII characters are present.
 * Fix security issues:
-	- Make (configurable) limit on the number of redirects
 	- Impose limitations on the imapflags extension regarding the number of
 	  set flags and the length of each flag name.
 	- Malicious/Broken binary can allocate large variable storage
diff --git a/src/lib-sieve/cmd-discard.c b/src/lib-sieve/cmd-discard.c
index 6a03eec59..24abee22e 100644
--- a/src/lib-sieve/cmd-discard.c
+++ b/src/lib-sieve/cmd-discard.c
@@ -127,7 +127,7 @@ static int cmd_discard_operation_execute
 	sieve_runtime_trace(renv, "DISCARD action");
 
 	return ( sieve_result_add_action
-		(renv, &act_discard, NULL, source_line, NULL) >= 0 );
+		(renv, &act_discard, NULL, source_line, NULL, 0) >= 0 );
 }
 
 /*
diff --git a/src/lib-sieve/cmd-redirect.c b/src/lib-sieve/cmd-redirect.c
index cfa79a329..816fad1d1 100644
--- a/src/lib-sieve/cmd-redirect.c
+++ b/src/lib-sieve/cmd-redirect.c
@@ -8,6 +8,7 @@
 #include "istream-header-filter.h"
 
 #include "sieve-common.h"
+#include "sieve-limits.h"
 #include "sieve-address.h"
 #include "sieve-commands.h"
 #include "sieve-code.h"
@@ -226,7 +227,7 @@ static int cmd_redirect_operation_execute
 	act->to_address = p_strdup(pool, str_c(redirect));
 	
 	ret = sieve_result_add_action
-		(renv, &act_redirect, slist, source_line, (void *) act);
+		(renv, &act_redirect, slist, source_line, (void *) act, sieve_max_redirects);
 	
 	return ( ret >= 0 );
 }
diff --git a/src/lib-sieve/ext-reject.c b/src/lib-sieve/ext-reject.c
index 836fe8d82..69521b338 100644
--- a/src/lib-sieve/ext-reject.c
+++ b/src/lib-sieve/ext-reject.c
@@ -243,7 +243,7 @@ static int ext_reject_operation_execute
 	act->reason = p_strdup(pool, str_c(reason));
 	
 	ret = sieve_result_add_action
-		(renv, &act_reject, slist, source_line, (void *) act);
+		(renv, &act_reject, slist, source_line, (void *) act, 0);
 	
 	return ( ret >= 0 );
 }
diff --git a/src/lib-sieve/plugins/vacation/cmd-vacation.c b/src/lib-sieve/plugins/vacation/cmd-vacation.c
index c11233c53..a639715b0 100644
--- a/src/lib-sieve/plugins/vacation/cmd-vacation.c
+++ b/src/lib-sieve/plugins/vacation/cmd-vacation.c
@@ -534,7 +534,7 @@ static int ext_vacation_operation_execute
 		sieve_coded_stringlist_read_all(addresses, pool, &(act->addresses));
 		
 	return ( sieve_result_add_action
-		(renv, &act_vacation, slist, source_line, (void *) act) >= 0 );
+		(renv, &act_vacation, slist, source_line, (void *) act, 0) >= 0 );
 }
 
 /*
diff --git a/src/lib-sieve/sieve-actions.c b/src/lib-sieve/sieve-actions.c
index 00a80f5f2..57b118b75 100644
--- a/src/lib-sieve/sieve-actions.c
+++ b/src/lib-sieve/sieve-actions.c
@@ -116,7 +116,7 @@ int sieve_act_store_add_to_result
 	act->folder = p_strdup(pool, folder);
 
 	return sieve_result_add_action(renv, &act_store, seffects, 
-		source_line, (void *) act);
+		source_line, (void *) act, 0);
 }
 
 /* Result verification */
diff --git a/src/lib-sieve/sieve-result.c b/src/lib-sieve/sieve-result.c
index 24d6f0b1e..e77828639 100644
--- a/src/lib-sieve/sieve-result.c
+++ b/src/lib-sieve/sieve-result.c
@@ -228,9 +228,10 @@ void sieve_result_add_implicit_side_effect
 int sieve_result_add_action
 (const struct sieve_runtime_env *renv,
 	const struct sieve_action *action, struct sieve_side_effects_list *seffects,
-	unsigned int source_line, void *context)		
+	unsigned int source_line, void *context, unsigned int instance_limit)		
 {
 	int ret = 0;
+	unsigned int instance_count = 0;
 	struct sieve_result *result = renv->result;
 	struct sieve_result_action *raction;
 	const char *location = sieve_error_script_location
@@ -242,6 +243,8 @@ int sieve_result_add_action
 		const struct sieve_action *oact = raction->action;
 		
 		if ( raction->action == action ) {
+			instance_count++;
+
 			/* Possible duplicate */
 			if ( action->check_duplicate != NULL ) {
 				if ( (ret=action->check_duplicate
@@ -265,11 +268,18 @@ int sieve_result_add_action
 		raction = raction->next;
 	}
 
-	/* Check policy limit on number of actions */
-	if ( result->action_count >= sieve_max_actions ) {
-		sieve_runtime_error(renv, location, "number of actions exceeds policy limit");
+	/* Check policy limit on total number of actions */
+	if ( sieve_max_actions > 0 && result->action_count >= sieve_max_actions ) {
+		sieve_runtime_error(renv, location, "total number of actions exceeds policy limit");
 		return -1;
 	}
+
+	/* Check policy limit on number of this class of actions */
+	if ( instance_limit > 0 && instance_count >= instance_limit ) {
+		sieve_runtime_error(renv, location, "number of %s actions exceeds policy limit",
+			action->name);
+		return -1;
+	}	
 		
 	/* Create new action object */
 	raction = p_new(result->pool, struct sieve_result_action, 1);
diff --git a/src/lib-sieve/sieve-result.h b/src/lib-sieve/sieve-result.h
index 9d3ad4aea..e9b232623 100644
--- a/src/lib-sieve/sieve-result.h
+++ b/src/lib-sieve/sieve-result.h
@@ -77,7 +77,7 @@ void sieve_result_add_implicit_side_effect
 int sieve_result_add_action
 (const struct sieve_runtime_env *renv,
 	const struct sieve_action *action, struct sieve_side_effects_list *seffects,
-	unsigned int source_line, void *context);
+	unsigned int source_line, void *context, unsigned int instance_limit);
 
 /*
  * Result execution
diff --git a/tests/execute/errors.svtest b/tests/execute/errors.svtest
index 60842f308..2ebc55026 100644
--- a/tests/execute/errors.svtest
+++ b/tests/execute/errors.svtest
@@ -45,7 +45,7 @@ test "Action conflicts: reject <-> redirect" {
 	}
 }
 
-test "Action limits" {
+test "Action limit" {
 	if not test_compile "errors/actions-limit.sieve" {
 		test_fail "compile failed";
 	}
@@ -58,7 +58,25 @@ test "Action limits" {
 		test_fail "too many runtime errors reported";
 	}
 	
-	if not test_error :index 1 :contains "number of actions exceeds policy limit"{
+	if not test_error :index 1 :contains "total number of actions exceeds policy limit"{
+		test_fail "unexpected error reported";
+	}
+}
+
+test "Redirect limit" {
+	if not test_compile "errors/redirect-limit.sieve" {
+		test_fail "compile failed";
+	}
+
+	if test_execute {
+		test_fail "execution should have failed";
+	}
+
+	if test_error :count "gt" :comparator "i;ascii-numeric" "1" {
+		test_fail "too many runtime errors reported";
+	}
+	
+	if not test_error :index 1 :contains "number of redirect actions exceeds policy limit"{
 		test_fail "unexpected error reported";
 	}
 }
diff --git a/tests/execute/errors/actions-limit.sieve b/tests/execute/errors/actions-limit.sieve
index 5f0421849..3ae33a306 100644
--- a/tests/execute/errors/actions-limit.sieve
+++ b/tests/execute/errors/actions-limit.sieve
@@ -16,20 +16,20 @@ fileinto "box13";
 fileinto "box14";
 fileinto "box15";
 fileinto "box16";
+fileinto "box17";
+fileinto "box18";
+fileinto "box19";
+fileinto "box20";
+fileinto "box21";
+fileinto "box22";
+fileinto "box23";
+fileinto "box24";
+fileinto "box25";
+fileinto "box26";
+fileinto "box27";
+fileinto "box28";
 redirect "address1@example.com";
 redirect "address2@example.com";
 redirect "address3@example.com";
 redirect "address4@example.com";
-redirect "address5@example.com";
-redirect "address6@example.com";
-redirect "address7@example.com";
-redirect "address8@example.com";
-redirect "address9@example.com";
-redirect "address10@example.com";
-redirect "address11@example.com";
-redirect "address12@example.com";
-redirect "address13@example.com";
-redirect "address14@example.com";
-redirect "address15@example.com";
-redirect "address16@example.com";
 keep;
diff --git a/tests/execute/errors/redirect-limit.sieve b/tests/execute/errors/redirect-limit.sieve
new file mode 100644
index 000000000..86cfda0c7
--- /dev/null
+++ b/tests/execute/errors/redirect-limit.sieve
@@ -0,0 +1,5 @@
+redirect "address1@example.com";
+redirect "address2@example.com";
+redirect "address3@example.com";
+redirect "address4@example.com";
+redirect "address5@example.com";
-- 
GitLab