Implemented policy limit on the maximum number of redirect actions in a result.

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

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 );
 }
 
 /*

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 );
 }

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 );
 }

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 );
 }
 
 /*

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 */

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);

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

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";
 	}
 }

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;

tests/execute/errors/redirect-limit.sieve

+redirect "address1@example.com";
+redirect "address2@example.com";
+redirect "address3@example.com";
+redirect "address4@example.com";
+redirect "address5@example.com";