Skip to content

Commit

Permalink
smtp: avoid creating empty transaction
Browse files Browse the repository at this point in the history
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 61f2e4e)
  • Loading branch information
catenacyber authored and victorjulien committed Feb 7, 2024
1 parent 2a2120e commit 83c5567
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 26 deletions.
34 changes: 9 additions & 25 deletions src/app-layer-smtp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down Expand Up @@ -1121,43 +1121,26 @@ 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,
FILE_NOMD5|FILE_NOMAGIC|FILE_USE_DETECT) == 0) {
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;
Expand All @@ -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;
Expand Down
7 changes: 6 additions & 1 deletion src/app-layer-smtp.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down

0 comments on commit 83c5567

Please sign in to comment.