From 540a6429d39ba1904c375b11bc12f28bec00a1c3 Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan@rename-it.nl>
Date: Sat, 26 Jul 2008 00:11:06 +0200
Subject: [PATCH] Improved the handling corrupt binaries further for the action
 commands.

---
 TODO                              |  4 ++--
 src/lib-sieve/cmd-keep.c          |  7 +++----
 src/lib-sieve/cmd-redirect.c      |  7 +++----
 src/lib-sieve/ext-fileinto.c      | 13 ++++++-------
 src/lib-sieve/ext-reject.c        | 13 +++++++++----
 src/lib-sieve/sieve-actions.c     |  1 -
 src/lib-sieve/sieve-binary.c      |  9 ---------
 src/lib-sieve/sieve-binary.h      |  2 --
 src/lib-sieve/sieve-code-dumper.c |  1 -
 src/lib-sieve/sieve-interpreter.c | 19 +++++++++++++------
 src/lib-sieve/sieve-interpreter.h |  2 +-
 11 files changed, 37 insertions(+), 41 deletions(-)

diff --git a/TODO b/TODO
index 150d2e69b..f27a1091d 100644
--- a/TODO
+++ b/TODO
@@ -11,10 +11,10 @@ Next (in order of descending priority/precedence):
       of the ADDRESS-PART argument specified.
     - Header test does not strip trailing whitespace
 * Code cleanup 
-* Make sure cmusieve can be replaced seamlessly with the new plugin.
 * Full security review. Enforce limits on number of created objects, script 
   size, execution time, etc...
-* Make simple test suite for the base functionality
+* Finish the test suite for the base functionality
+* Make sure cmusieve can be replaced seamlessly with the new plugin.
 
 * ## MAKE A FIRST RELEASE ##
 
diff --git a/src/lib-sieve/cmd-keep.c b/src/lib-sieve/cmd-keep.c
index d7881b59d..333578947 100644
--- a/src/lib-sieve/cmd-keep.c
+++ b/src/lib-sieve/cmd-keep.c
@@ -107,10 +107,9 @@ static int cmd_keep_operation_execute
 	}
 	
 	/* Optional operands (side effects only) */
-	if ( !sieve_interpreter_handle_optional_operands(renv, address, &slist) ) {
-		sieve_runtime_trace_error(renv, "invalid optional operands");
-		return SIEVE_EXEC_BIN_CORRUPT;
-	}
+	if ( (ret=sieve_interpreter_handle_optional_operands
+		(renv, address, &slist)) <= 0 ) 
+		return ret;
 
 	sieve_runtime_trace(renv, "KEEP action");
 	
diff --git a/src/lib-sieve/cmd-redirect.c b/src/lib-sieve/cmd-redirect.c
index 5b2e4a688..08a882c72 100644
--- a/src/lib-sieve/cmd-redirect.c
+++ b/src/lib-sieve/cmd-redirect.c
@@ -204,10 +204,9 @@ static int cmd_redirect_operation_execute
 	}
 
 	/* Optional operands (side effects) */
-	if ( !sieve_interpreter_handle_optional_operands(renv, address, &slist) ) {
-		sieve_runtime_trace_error(renv, "invalid optional operands");
-		return SIEVE_EXEC_BIN_CORRUPT;
-	}
+	if ( (ret=sieve_interpreter_handle_optional_operands
+		(renv, address, &slist)) <= 0 )
+		return ret;
 
 	/* Read the address */
 	if ( !sieve_opr_string_read(renv, address, &redirect) ) {
diff --git a/src/lib-sieve/ext-fileinto.c b/src/lib-sieve/ext-fileinto.c
index d5ff69726..9e26739c4 100644
--- a/src/lib-sieve/ext-fileinto.c
+++ b/src/lib-sieve/ext-fileinto.c
@@ -153,13 +153,10 @@ static bool ext_fileinto_operation_dump
         return FALSE;
 
 	if ( !sieve_code_dumper_print_optional_operands(denv, address) ) {
-		sieve_binary_corrupt(denv->sbin, 
-			"FILEINTO: failed to dump optional operands");
 		return FALSE;
 	}
 
 	if ( !sieve_opr_string_dump(denv, address) ) {
-		sieve_binary_corrupt(denv->sbin, "FILEINTO: failed to dump string operand");
 		return FALSE;
 	}
 	
@@ -180,14 +177,16 @@ static int ext_fileinto_operation_execute
 	int ret = 0;
 
 	/* Source line */
-    if ( !sieve_code_source_line_read(renv, address, &source_line) )
+    if ( !sieve_code_source_line_read(renv, address, &source_line) ) {
+		sieve_runtime_trace_error(renv, "invalid source line");
         return SIEVE_EXEC_BIN_CORRUPT;
+	}
 	
-	if ( !sieve_interpreter_handle_optional_operands(renv, address, &slist) )
-		return SIEVE_EXEC_BIN_CORRUPT;
+	if ( (ret=sieve_interpreter_handle_optional_operands(renv, address, &slist)) <= 0 )
+		return ret;
 
 	if ( !sieve_opr_string_read(renv, address, &folder) ) {
-		sieve_binary_corrupt(renv->sbin, "FILEINTO: failed to read string operand");
+		sieve_runtime_trace_error(renv, "invalid folder operand");
 		return SIEVE_EXEC_BIN_CORRUPT;
 	}
 
diff --git a/src/lib-sieve/ext-reject.c b/src/lib-sieve/ext-reject.c
index 8511e59f6..6b7f31ae8 100644
--- a/src/lib-sieve/ext-reject.c
+++ b/src/lib-sieve/ext-reject.c
@@ -217,16 +217,21 @@ static int ext_reject_operation_execute
 	int ret;
 
 	/* Source line */
-    if ( !sieve_code_source_line_read(renv, address, &source_line) )
+    if ( !sieve_code_source_line_read(renv, address, &source_line) ) {
+		sieve_runtime_trace_error(renv, "invalid source line");
         return SIEVE_EXEC_BIN_CORRUPT;
+	}
 	
 	/* Optional operands (side effects) */
-	if ( !sieve_interpreter_handle_optional_operands(renv, address, &slist) )
-		return SIEVE_EXEC_BIN_CORRUPT;
+	if ( (ret=sieve_interpreter_handle_optional_operands
+		(renv, address, &slist)) <= 0 )
+		return ret;
 
 	/* Read rejection reason */
-	if ( !sieve_opr_string_read(renv, address, &reason) )
+	if ( !sieve_opr_string_read(renv, address, &reason) ) {
+		sieve_runtime_trace_error(renv, "invalid reason operand");
 		return SIEVE_EXEC_BIN_CORRUPT;
+	}
 
 	sieve_runtime_trace(renv, "REJECT action (\"%s\")", str_c(reason));
 
diff --git a/src/lib-sieve/sieve-actions.c b/src/lib-sieve/sieve-actions.c
index 3f5d20084..f1139461c 100644
--- a/src/lib-sieve/sieve-actions.c
+++ b/src/lib-sieve/sieve-actions.c
@@ -48,7 +48,6 @@ bool sieve_opr_side_effect_dump
 	if ( seffect->dump_context != NULL ) {
 		sieve_code_descend(denv);
 		if ( !seffect->dump_context(seffect, denv, address) ) {
-			sieve_binary_corrupt(denv->sbin, "failed to read side effect context");
 			return FALSE;	
 		}
 		sieve_code_ascend(denv);
diff --git a/src/lib-sieve/sieve-binary.c b/src/lib-sieve/sieve-binary.c
index cf7cf4658..2dc816644 100644
--- a/src/lib-sieve/sieve-binary.c
+++ b/src/lib-sieve/sieve-binary.c
@@ -274,15 +274,6 @@ bool sieve_binary_script_older
 	return ( sieve_script_older(script, sbin->file->st.st_mtime) );
 }
 
-void sieve_binary_corrupt(struct sieve_binary *sbin, const char *fmt, ...)
-{
-	va_list args;
-	
-	va_start(args, fmt);
-	printf("BINARY CORRUPT: %s.\n", t_strdup_vprintf(fmt, args));
-	va_end(args);
-}
-
 /* 
  * Block management 
  */
diff --git a/src/lib-sieve/sieve-binary.h b/src/lib-sieve/sieve-binary.h
index a278d4900..4ccc26aa6 100644
--- a/src/lib-sieve/sieve-binary.h
+++ b/src/lib-sieve/sieve-binary.h
@@ -18,8 +18,6 @@ const char *sieve_binary_path(struct sieve_binary *sbin);
 bool sieve_binary_script_older
 	(struct sieve_binary *sbin, struct sieve_script *script);
 
-void sieve_binary_corrupt(struct sieve_binary *sbin, const char *fmt, ...);
-
 void sieve_binary_activate(struct sieve_binary *sbin);
 
 bool sieve_binary_save
diff --git a/src/lib-sieve/sieve-code-dumper.c b/src/lib-sieve/sieve-code-dumper.c
index 619a0f734..8f1b55a61 100644
--- a/src/lib-sieve/sieve-code-dumper.c
+++ b/src/lib-sieve/sieve-code-dumper.c
@@ -117,7 +117,6 @@ bool sieve_code_dumper_print_optional_operands
 		
 		while ( opt_code != 0 ) {			
 			if ( !sieve_operand_optional_read(denv->sbin, address, &opt_code) ) {
-				sieve_binary_corrupt(denv->sbin, "failed to read optional operand");
 				return FALSE;
 			}
 
diff --git a/src/lib-sieve/sieve-interpreter.c b/src/lib-sieve/sieve-interpreter.c
index edb5adc6d..73e2b5183 100644
--- a/src/lib-sieve/sieve-interpreter.c
+++ b/src/lib-sieve/sieve-interpreter.c
@@ -293,7 +293,7 @@ bool sieve_interpreter_get_test_result
 
 /* Operations and operands */
 
-bool sieve_interpreter_handle_optional_operands
+int sieve_interpreter_handle_optional_operands
 	(const struct sieve_runtime_env *renv, sieve_size_t *address,
 		struct sieve_side_effects_list **list)
 {
@@ -301,8 +301,10 @@ bool sieve_interpreter_handle_optional_operands
 	
 	if ( sieve_operand_optional_present(renv->sbin, address) ) {
 		while ( opt_code != 0 ) {
-			if ( !sieve_operand_optional_read(renv->sbin, address, &opt_code) )
-				return FALSE;
+			if ( !sieve_operand_optional_read(renv->sbin, address, &opt_code) ) {
+				sieve_runtime_trace_error(renv, "invalid optional operand");
+				return SIEVE_EXEC_BIN_CORRUPT;
+			}
 
 			if ( opt_code == SIEVE_OPT_SIDE_EFFECT ) {
 				void *context = NULL;
@@ -313,12 +315,17 @@ bool sieve_interpreter_handle_optional_operands
 				const struct sieve_side_effect *seffect = 
 					sieve_opr_side_effect_read(renv, address);
 
-				if ( seffect == NULL ) return FALSE;
+				if ( seffect == NULL ) {
+					sieve_runtime_trace_error(renv, "invalid side effect operand");
+					return SIEVE_EXEC_BIN_CORRUPT;
+				}
 			
 				if ( list != NULL ) {
 					if ( seffect->read_context != NULL && !seffect->read_context
-						(seffect, renv, address, &context) )
-						return FALSE;
+						(seffect, renv, address, &context) ) {
+						sieve_runtime_trace_error(renv, "invalid side effect context");
+						return SIEVE_EXEC_BIN_CORRUPT;
+					}
 				
 					sieve_side_effects_list_add(*list, seffect, context);
 				}
diff --git a/src/lib-sieve/sieve-interpreter.h b/src/lib-sieve/sieve-interpreter.h
index f8c5d8b02..86ab5d446 100644
--- a/src/lib-sieve/sieve-interpreter.h
+++ b/src/lib-sieve/sieve-interpreter.h
@@ -105,7 +105,7 @@ const void *sieve_interpreter_extension_get_context
 
 /* Opcodes and operands */
 	
-bool sieve_interpreter_handle_optional_operands
+int sieve_interpreter_handle_optional_operands
 	(const struct sieve_runtime_env *renv, sieve_size_t *address,
 		struct sieve_side_effects_list **list);
 
-- 
GitLab