lib-smtp: smtp-server-transaction - Make sure current data command is recorded as...

This prevents crashes when replies are sent early. This also prevents crashes
when invalid DATA commands are sent in succession.

diff --git a/src/lib-smtp/smtp-server-cmd-data.c b/src/lib-smtp/smtp-server-cmd-data.c
index 8498c1707e8e3e3e2ac7f824afc10fd6ea40106e..01716b1062e02c90c873880f8e638ff728c7077b 100644 (file)
--- a/src/lib-smtp/smtp-server-cmd-data.c
+++ b/src/lib-smtp/smtp-server-cmd-data.c
@@ -354,14 +354,14 @@ cmd_data_next(struct smtp_server_cmd_ctx *cmd,
 
        e_debug(cmd->event, "Command is next to be replied");
 
+       if (trans != NULL)
+               smtp_server_transaction_data_command(trans, cmd);
+
        /* check whether we have had successful mail and rcpt commands */
        if (!smtp_server_connection_data_check_state(cmd))
                return;
 
        if (data_cmd->chunk_last) {
-               /* This is the last chunk */
-               smtp_server_transaction_data_command(trans, cmd);
-
                /* LMTP 'DATA' and 'BDAT LAST' commands need to send more than
                   one reply per recipient */
                if (HAS_ALL_BITS(trans->flags,
@@ -468,7 +468,6 @@ cmd_data_start(struct smtp_server_cmd_ctx *cmd,
        i_assert(conn->state.pending_mail_cmds == 0 &&
                conn->state.pending_rcpt_cmds == 0);
 
-       /* this is the one and only data command */
        if (trans != NULL)
                smtp_server_transaction_data_command(trans, cmd);
 
@@ -561,6 +560,7 @@ int smtp_server_connection_data_chunk_add(struct smtp_server_cmd_ctx *cmd,
        bool client_input)
 {
        struct smtp_server_connection *conn = cmd->conn;
+       struct smtp_server_transaction *trans = conn->state.trans;
        const struct smtp_server_settings *set = &conn->set;
        struct smtp_server_command *command = cmd->cmd;
        struct cmd_data_context *data_cmd = command->data;
@@ -568,6 +568,9 @@ int smtp_server_connection_data_chunk_add(struct smtp_server_cmd_ctx *cmd,
 
        i_assert(data_cmd != NULL);
 
+       if (trans != NULL)
+               smtp_server_transaction_data_command(trans, cmd);
+
        if (!smtp_server_connection_data_check_state(cmd))
                return -1;
 

diff --git a/src/lib-smtp/smtp-server-recipient.c b/src/lib-smtp/smtp-server-recipient.c
index aff59c24b4d6e407daac984239ecab0607ccc848..d12aa39c3fc8cf053810665442148018d3b42921 100644 (file)
--- a/src/lib-smtp/smtp-server-recipient.c
+++ b/src/lib-smtp/smtp-server-recipient.c
@@ -169,7 +169,6 @@ void smtp_server_recipient_denied(struct smtp_server_recipient *rcpt,
 void smtp_server_recipient_data_command(struct smtp_server_recipient *rcpt,
                                        struct smtp_server_cmd_ctx *cmd)
 {
-       i_assert(rcpt->cmd == NULL);
        rcpt->cmd = cmd;
 }
 

diff --git a/src/lib-smtp/smtp-server-transaction.c b/src/lib-smtp/smtp-server-transaction.c
index a66bc125d7f9edf6c80386e067f3eba9275fd126..206b6bdb9b027aeb734f79a01d6bafaeeaaad1ee 100644 (file)
--- a/src/lib-smtp/smtp-server-transaction.c
+++ b/src/lib-smtp/smtp-server-transaction.c
@@ -167,10 +167,6 @@ void smtp_server_transaction_data_command(struct smtp_server_transaction *trans,
 {
        struct smtp_server_recipient *const *rcptp;
 
-       if (trans->cmd != NULL) {
-               i_assert(cmd == trans->cmd);
-               return;
-       }
        trans->cmd = cmd;
 
        if (!array_is_created(&trans->rcpt_to))

diff --git a/src/lib-smtp/test-smtp-server-errors.c b/src/lib-smtp/test-smtp-server-errors.c
index 44fc25d353ad2d89b04aefd9c9e54aab4d2beb7c..0a85e90f04c800bbc82356e916a3b90debafb22c 100644 (file)
--- a/src/lib-smtp/test-smtp-server-errors.c
+++ b/src/lib-smtp/test-smtp-server-errors.c
@@ -2496,6 +2496,93 @@ static void test_data_no_rcpt(void)
        test_end();
 }
 
+/*
+ * Bad pipelined DATA
+ */
+
+/* client */
+
+static void test_bad_pipelined_data_connected(struct client_connection *conn)
+{
+       o_stream_nsend_str(conn->conn.output,
+                          "MAIL FROM:<senderp@example.com>\r\n"
+                          "RCPT TO:<<recipient1@example.com>\r\n"
+                          "DATA\r\n"
+                          "FROP!\r\n"
+                          "DATA\r\n"
+                          "FROP!\r\n"
+                          ".\r\n"
+                          "QUIT\r\n");
+}
+
+static void test_client_bad_pipelined_data(unsigned int index)
+{
+       test_client_connected = test_bad_pipelined_data_connected;
+       test_client_run(index);
+}
+
+/* server */
+
+static void
+test_server_bad_pipelined_data_trans_free(
+       void *conn_ctx  ATTR_UNUSED,
+       struct smtp_server_transaction *trans ATTR_UNUSED)
+{
+       io_loop_stop(ioloop);
+}
+
+static int
+test_server_bad_pipelined_data_rcpt(
+       void *conn_ctx ATTR_UNUSED, struct smtp_server_cmd_ctx *cmd ATTR_UNUSED,
+       struct smtp_server_recipient *rcpt ATTR_UNUSED)
+{
+       /* not supposed to get here */
+       i_assert(FALSE);
+       return 1;
+}
+
+static int
+test_server_bad_pipelined_data_data_begin(
+       void *conn_ctx ATTR_UNUSED, struct smtp_server_cmd_ctx *cmd ATTR_UNUSED,
+       struct smtp_server_transaction *trans ATTR_UNUSED,
+       struct istream *data_input ATTR_UNUSED)
+{
+       /* not supposed to get here */
+       i_assert(FALSE);
+       return 1;
+}
+
+static void
+test_server_bad_pipelined_data(const struct smtp_server_settings *server_set)
+{
+       server_callbacks.conn_trans_free =
+               test_server_bad_pipelined_data_trans_free;
+       server_callbacks.conn_cmd_rcpt =
+               test_server_bad_pipelined_data_rcpt;
+       server_callbacks.conn_cmd_data_begin =
+               test_server_bad_pipelined_data_data_begin;
+       test_server_run(server_set);
+}
+
+/* test */
+
+static void test_bad_pipelined_data(void)
+{
+       struct smtp_server_settings smtp_server_set;
+
+       test_server_defaults(&smtp_server_set);
+       smtp_server_set.capabilities =
+               SMTP_CAPABILITY_BINARYMIME | SMTP_CAPABILITY_CHUNKING;
+       smtp_server_set.max_client_idle_time_msecs = 1000;
+       smtp_server_set.max_recipients = 10;
+
+       test_begin("Bad pipelined DATA");
+       test_run_client_server(&smtp_server_set,
+                              test_server_bad_pipelined_data,
+                              test_client_bad_pipelined_data, 1);
+       test_end();
+}
+
 /*
  * DATA with BINARYMIME
  */
@@ -2840,6 +2927,7 @@ static void (*const test_functions[])(void) = {
        test_too_many_recipients,
        test_data_no_mail,
        test_data_no_rcpt,
+       test_bad_pipelined_data,
        test_data_binarymime,
        test_mail_broken_path,
        NULL