From 78f5952ca43579854264f1a87e29268242db6dcc Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan.bosch@dovecot.fi>
Date: Tue, 11 Dec 2018 17:29:18 +0100
Subject: [PATCH] lib-sieve: redirect action: Implement additional protection
 against mail loops.

Also check the X-Sieve-Redirected-From header for our own e-mail addresses. This
header is added by the redirect action itself and in a mail loop it would see
that same header with that same content. This is less reliable than the other
mail loop detection (sender may set such a header), so, unlike the existing loop
detection based on the duplicate db, the implicit keep is not canceled when the
new loop detection is triggered.
---
 src/lib-sieve/cmd-redirect.c |  66 ++++++++++++++++++++++-
 tests/execute/smtp.svtest    | 101 +++++++++++++++++++++++++++++++++++
 2 files changed, 166 insertions(+), 1 deletion(-)

diff --git a/src/lib-sieve/cmd-redirect.c b/src/lib-sieve/cmd-redirect.c
index 0f4e7db1a..200d167b4 100644
--- a/src/lib-sieve/cmd-redirect.c
+++ b/src/lib-sieve/cmd-redirect.c
@@ -281,7 +281,7 @@ act_redirect_send(const struct sieve_action_exec_env *aenv, struct mail *mail,
 	ATTR_NULL(4)
 {
 	static const char *hide_headers[] =
-		{ "Return-Path", "X-Sieve", "X-Sieve-Redirected-From" };
+		{ "Return-Path" };
 	struct sieve_instance *svinst = aenv->svinst;
 	struct sieve_message_context *msgctx = aenv->msgctx;
 	const struct sieve_script_env *senv = aenv->scriptenv;
@@ -460,6 +460,55 @@ act_redirect_get_duplicate_id(struct act_redirect_context *ctx,
 	return SIEVE_EXEC_OK;
 }
 
+static int
+act_redirect_check_loop_header(const struct sieve_action_exec_env *aenv,
+			       struct mail *mail, bool *loop_detected_r)
+{
+	struct sieve_message_context *msgctx = aenv->msgctx;
+	const char *const *headers;
+	const char *recipient, *user_email;
+	const struct smtp_address *addr;
+	int ret;
+
+	*loop_detected_r = FALSE;
+
+	ret = mail_get_headers(mail, "x-sieve-redirected-from", &headers);
+	if (ret < 0 ) {
+		return sieve_result_mail_error(
+			aenv, mail, "redirect action: "
+			"failed to read header field "
+			"`x-sieve-redirected-from'");
+	}
+
+	if (ret == 0)
+		return SIEVE_EXEC_OK;
+
+	recipient = user_email = NULL;
+	if ((aenv->flags & SIEVE_EXECUTE_FLAG_NO_ENVELOPE) == 0) {
+		addr = sieve_message_get_final_recipient(msgctx);
+		if (addr != NULL)
+			recipient = smtp_address_encode(addr);
+	}
+	addr = sieve_get_user_email(aenv->svinst);
+	if (addr != NULL)
+		user_email = smtp_address_encode(addr);
+
+	while (*headers != NULL) {
+		const char *header = t_str_trim(*headers, " \t\r\n");
+		if (strcmp(header, recipient) == 0) {
+			*loop_detected_r = TRUE;
+			break;
+		}
+		if (strcmp(header, user_email) == 0) {
+			*loop_detected_r = TRUE;
+			break;
+		}
+		headers++;
+	}
+
+	return SIEVE_EXEC_OK;
+}
+
 static int
 act_redirect_commit(const struct sieve_action *action,
 		    const struct sieve_action_exec_env *aenv,
@@ -475,6 +524,7 @@ act_redirect_commit(const struct sieve_action *action,
 	const struct sieve_script_env *senv = aenv->scriptenv;
 	const char *msg_id = msgdata->id, *new_msg_id = NULL;
 	const char *dupeid = NULL;
+	bool loop_detected = FALSE;
 	int ret;
 
 	/*
@@ -500,6 +550,20 @@ act_redirect_commit(const struct sieve_action *action,
 		return SIEVE_EXEC_OK;
 	}
 
+	/* Check whether we've seen this message before based on added headers
+	 */
+	ret = act_redirect_check_loop_header(aenv, mail, &loop_detected);
+	if (ret != SIEVE_EXEC_OK)
+		return ret;
+	if (loop_detected) {
+		sieve_result_global_log(
+			aenv, "redirect action: "
+			"not forwarding message to <%s>: "
+			"the `x-sieve-redirected-from' header indicates a mail loop",
+			smtp_address_encode(ctx->to_address));
+		return SIEVE_EXEC_OK;
+	}
+
 	/*
 	 * Try to forward the message
 	 */
diff --git a/tests/execute/smtp.svtest b/tests/execute/smtp.svtest
index 4e90c1215..2c7d2e6cb 100644
--- a/tests/execute/smtp.svtest
+++ b/tests/execute/smtp.svtest
@@ -346,3 +346,104 @@ test "Redirect from [user email]" {
 	}
 }
 
+/*
+ * Redirect mail loop (sieve_user_email)
+ */
+
+test_result_reset;
+test_set "message" text:
+X-Sieve-Redirected-From: t.sirainen@example.net
+From: stephan@example.org
+To: tss@example.net
+Subject: Frop!
+
+Frop!
+.
+;
+test_set "envelope.from" "sirius@example.org";
+test_set "envelope.to" "timo@example.net";
+test_set "envelope.orig_to" "tss@example.net";
+
+test_config_set "sieve_redirect_envelope_from" "user_email";
+test_config_set "sieve_user_email" "t.sirainen@example.net";
+test_config_reload;
+
+test "Redirect mail loop (sieve_user_email)" {
+	redirect "cras@example.net";
+
+	if not test_result_execute {
+		test_fail "failed to execute redirect";
+	}
+
+	if test_message :smtp 0 {
+		test_fail "failed to recognize mail loop";
+	}
+}
+
+/*
+ * Redirect mail loop (final recipient)
+ */
+
+test_result_reset;
+test_set "message" text:
+X-Sieve-Redirected-From: timo@example.net
+From: stephan@example.org
+To: tss@example.net
+Subject: Frop!
+
+Frop!
+.
+;
+test_set "envelope.from" "sirius@example.org";
+test_set "envelope.to" "timo@example.net";
+test_set "envelope.orig_to" "tss@example.net";
+
+test_config_reload;
+
+test "Redirect mail loop (final recipient)" {
+	redirect "cras@example.net";
+
+	if not test_result_execute {
+		test_fail "failed to execute redirect";
+	}
+
+	if test_message :smtp 0 {
+		test_fail "failed to recognize mail loop";
+	}
+}
+
+/*
+ * Redirect mail loop (multiple headers)
+ */
+
+test_result_reset;
+test_set "message" text:
+X-Sieve-Redirected-From: stephan@example.net
+From: stephan@example.org
+To: tss@example.net
+Subject: Frop!
+X-Sieve-Redirected-From: t.sirainen@example.net
+X-Sieve-Redirected-From: t.sirainen@example.com
+
+Frop!
+.
+;
+test_set "envelope.from" "sirius@example.org";
+test_set "envelope.to" "timo@example.net";
+test_set "envelope.orig_to" "tss@example.net";
+
+test_config_set "sieve_redirect_envelope_from" "user_email";
+test_config_set "sieve_user_email" "t.sirainen@example.net";
+test_config_reload;
+
+test "Redirect mail loop (sieve_user_email)" {
+	redirect "cras@example.net";
+
+	if not test_result_execute {
+		test_fail "failed to execute redirect";
+	}
+
+	if test_message :smtp 0 {
+		test_fail "failed to recognize mail loop";
+	}
+}
-- 
GitLab