From: Stephan Bosch Date: Mon, 12 Apr 2021 20:43:57 +0000 (+0200) Subject: lib-smtp: smtp-server-transaction - Make sure current data command is recorded as... X-Git-Tag: 2.3.16~264 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=02de37f6c22be7f1a0940d460d2b2abaeff8bb79;p=thirdparty%2Fdovecot%2Fcore.git lib-smtp: smtp-server-transaction - Make sure current data command is recorded as soon as possible. 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 8498c1707e..01716b1062 100644 --- 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 aff59c24b4..d12aa39c3f 100644 --- 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 a66bc125d7..206b6bdb9b 100644 --- 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 44fc25d353..0a85e90f04 100644 --- 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:\r\n" + "RCPT TO:<\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