smtp: avoid creating empty transaction

Ticket: 6477

So as to avoid ending up with too many empty transactions.

This happens when Suricata sees a DATA command in the current
transaction but did not have a confirmation response for it.
Then, if Suricata receives another DATA command, it will
create another new transaction, even if the previous one
is empty. And so, a malicious client can create many empty
transactions by just sending a repeated amount of DATA commands
without having a confirmation code for them.

Suricata cannot use state->current_command == SMTP_COMMAND_DATA
to prevent this attack and needs to resort to a new boolean
is_data because the malicious client may send another dummy command
after each DATA command.

This patch leaves only one call to SMTPTransactionCreate

(cherry picked from commit 61f2e4e1e5b34dfd8ae44d1c15253e2da65f6e6a)

diff --git a/src/app-layer-smtp.c b/src/app-layer-smtp.c
index 4d85bb370a265f48b8083c656ca8f2dfe741a489..29a4cbbcafde1d22582c3538460d30bffef7d8e4 100644 (file)
--- a/src/app-layer-smtp.c
+++ b/src/app-layer-smtp.c
@@ -722,7 +722,7 @@ static inline void SMTPTransactionComplete(SMTPState *state)
 {
     DEBUG_VALIDATE_BUG_ON(state->curr_tx == NULL);
     if (state->curr_tx)
-        state->curr_tx->done = 1;
+        state->curr_tx->done = true;
 }
 
 /**
@@ -1121,24 +1121,18 @@ static int SMTPProcessRequest(SMTPState *state, Flow *f,
         } else if (state->current_line_len >= 4 &&
                    SCMemcmpLowercase("data", state->current_line, 4) == 0) {
             state->current_command = SMTP_COMMAND_DATA;
-            if (smtp_config.raw_extraction) {
+            if (state->curr_tx->is_data) {
+                // We did not receive a confirmation from server
+                // And now the sends a next DATA
+                SMTPSetEvent(state, SMTP_DECODER_EVENT_UNPARSABLE_CONTENT);
+                SCReturnInt(0);
+            } else if (smtp_config.raw_extraction) {
                 const char *msgname = "rawmsg"; /* XXX have a better name */
                 if (state->files_ts == NULL)
                     state->files_ts = FileContainerAlloc();
                 if (state->files_ts == NULL) {
                     return -1;
                 }
-                if (state->tx_cnt > 1 && !state->curr_tx->done) {
-                    // we did not close the previous tx, set error
-                    SMTPSetEvent(state, SMTP_DECODER_EVENT_UNPARSABLE_CONTENT);
-                    FileCloseFile(state->files_ts, NULL, 0, FILE_TRUNCATED);
-                    tx = SMTPTransactionCreate(state);
-                    if (tx == NULL)
-                        return -1;
-                    state->curr_tx = tx;
-                    TAILQ_INSERT_TAIL(&state->tx_list, tx, next);
-                    tx->tx_id = state->tx_cnt++;
-                }
                 if (FileOpenFileWithId(state->files_ts, &smtp_config.sbcfg,
                         state->file_track_id++,
                         (uint8_t*) msgname, strlen(msgname), NULL, 0,
@@ -1146,18 +1140,7 @@ static int SMTPProcessRequest(SMTPState *state, Flow *f,
                     SMTPNewFile(state->curr_tx, state->files_ts->tail);
                 }
             } else if (smtp_config.decode_mime) {
-                if (tx->mime_state) {
-                    /* We have 2 chained mails and did not detect the end
-                     * of first one. So we start a new transaction. */
-                    tx->mime_state->state_flag = PARSE_ERROR;
-                    SMTPSetEvent(state, SMTP_DECODER_EVENT_UNPARSABLE_CONTENT);
-                    tx = SMTPTransactionCreate(state);
-                    if (tx == NULL)
-                        return -1;
-                    state->curr_tx = tx;
-                    TAILQ_INSERT_TAIL(&state->tx_list, tx, next);
-                    tx->tx_id = state->tx_cnt++;
-                }
+                DEBUG_VALIDATE_BUG_ON(tx->mime_state);
                 tx->mime_state = MimeDecInitParser(f, SMTPProcessDataChunk);
                 if (tx->mime_state == NULL) {
                     return MIME_DEC_ERR_MEM;
@@ -1173,6 +1156,7 @@ static int SMTPProcessRequest(SMTPState *state, Flow *f,
                     tx->msg_tail = tx->mime_state->msg;
                 }
             }
+            state->curr_tx->is_data = true;
             /* Enter immediately data mode without waiting for server reply */
             if (state->parser_state & SMTP_PARSER_STATE_PIPELINING_SERVER) {
                 state->parser_state |= SMTP_PARSER_STATE_COMMAND_DATA_MODE;

diff --git a/src/app-layer-smtp.h b/src/app-layer-smtp.h
index 7d010cced5aac69c304224ea793a2529607ef0b8..03e8f90eefbf7e843930ee0d6c930adc4bf78098 100644 (file)
--- a/src/app-layer-smtp.h
+++ b/src/app-layer-smtp.h
@@ -76,7 +76,12 @@ typedef struct SMTPTransaction_ {
 
     AppLayerTxData tx_data;
 
-    int done;
+    /** the tx is complete and can be logged and cleaned */
+    bool done;
+    /** the tx has seen a DATA command */
+    // another DATA command within the same context
+    // will trigger an app-layer event.
+    bool is_data;
     /** the first message contained in the session */
     MimeDecEntity *msg_head;
     /** the last message contained in the session */