From: Shivani Bhardwaj Date: Mon, 14 Feb 2022 11:23:52 +0000 (+0530) Subject: smtp: fix indefinite buffering if no LF in line X-Git-Tag: suricata-7.0.0-beta1~709 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=078c251deacc78b8abb40c5ab89d19c29e31bdf8;p=thirdparty%2Fsuricata.git smtp: fix indefinite buffering if no LF in line Issue ----- So far, with the SMTP parser, we would buffer data up until an LF char was found indicating the end of one line. This would happen in case of fragmented data where a line might come broken into multiple chunks. This was problematic if there was a really long line without any LF character. It would mean that we'd keep buffering data up until we encounter one such LF char which may be many many bytes of data later. Fix --- Fix this issue by setting an upper limit of 4KB on the buffering of lines. If the limit is reached then we save the data into current line and process it as if it were a regular request/response up until 4KB only. Any data after 4KB is discarded up until there is a new LF char in the received input. Cases ----- 1. Fragmentation The limit is enforced for any cases where a line of >= 4KB comes as diff fragments that are each/some < 4KB. 2. Single too long line The limit is also enforced for any cases where a single line exceeds the limit of buffer. Reported by Victor Julien. Ticket 5023 --- diff --git a/src/app-layer-smtp.c b/src/app-layer-smtp.c index 68a132edf4..f0142b326b 100644 --- a/src/app-layer-smtp.c +++ b/src/app-layer-smtp.c @@ -109,6 +109,8 @@ #define SMTP_EHLO_EXTENSION_DSN #define SMTP_EHLO_EXTENSION_STARTTLS #define SMTP_EHLO_EXTENSION_8BITMIME +/* Limit till the data would be buffered in current line */ +#define SMTP_LINE_BUFFER_LIMIT 4096 SCEnumCharMap smtp_decoder_event_table[] = { { "INVALID_REPLY", SMTP_DECODER_EVENT_INVALID_REPLY }, @@ -626,13 +628,26 @@ static AppLayerResult SMTPGetLine(SMTPState *state) uint8_t *lf_idx = memchr(state->input + state->consumed, 0x0a, state->input_len); if (lf_idx == NULL) { + if (!state->discard_till_lf && state->input_len >= SMTP_LINE_BUFFER_LIMIT) { + state->current_line = state->input; + state->current_line_len = SMTP_LINE_BUFFER_LIMIT; + state->current_line_delimiter_len = 0; + SCReturnStruct(APP_LAYER_OK); + } SCReturnStruct(APP_LAYER_INCOMPLETE(state->consumed, state->input_len + 1)); } else { uint32_t o_consumed = state->consumed; state->consumed = lf_idx - state->input + 1; state->current_line_len = state->consumed - o_consumed; - state->current_line = state->input + o_consumed; state->input_len -= state->current_line_len; + DEBUG_VALIDATE_BUG_ON((state->consumed + state->input_len) != state->orig_input_len); + if (state->discard_till_lf) { + // Whatever came in with first LF should also get discarded + state->discard_till_lf = false; + state->current_line_len = 0; + SCReturnStruct(APP_LAYER_OK); + } + state->current_line = state->input + o_consumed; if (state->consumed >= 2 && state->input[state->consumed - 2] == 0x0D) { state->current_line_delimiter_len = 2; state->current_line_len -= 2; @@ -839,6 +854,11 @@ static int SMTPProcessReply(SMTPState *state, Flow *f, { SCEnter(); + /* Line with just LF */ + if (state->current_line_len == 0 && state->consumed == 1) { + return 0; // to continue processing further + } + /* the reply code has to contain at least 3 bytes, to hold the 3 digit * reply code */ if (state->current_line_len < 3) { @@ -1093,6 +1113,11 @@ static int SMTPProcessRequest(SMTPState *state, Flow *f, SCEnter(); SMTPTransaction *tx = state->curr_tx; + /* Line with just LF */ + if (state->current_line_len == 0 && state->consumed == 1) { + return 0; // to continue processing further + } + if (state->curr_tx == NULL || (state->curr_tx->done && !NoNewTx(state))) { tx = SMTPTransactionCreate(); if (tx == NULL) @@ -1265,6 +1290,7 @@ static AppLayerResult SMTPParse(uint8_t direction, Flow *f, SMTPState *state, } state->input = input; + state->orig_input_len = input_len; state->input_len = input_len; state->consumed = 0; state->direction = direction; @@ -1273,18 +1299,37 @@ static AppLayerResult SMTPParse(uint8_t direction, Flow *f, SMTPState *state, /* toserver */ if (direction == 0) { while (res.status == 0) { - if (SMTPProcessRequest(state, f, pstate) == -1) - SCReturnStruct(APP_LAYER_ERROR); - res = SMTPGetLine(state); + if (!state->discard_till_lf) { + if ((state->current_line_len > 0) && (SMTPProcessRequest(state, f, pstate) == -1)) + SCReturnStruct(APP_LAYER_ERROR); + if (state->current_line_delimiter_len == 0 && + state->current_line_len == SMTP_LINE_BUFFER_LIMIT) { + state->discard_till_lf = true; + state->consumed = state->input_len + 1; // For the newly found LF + SMTPSetEvent(state, SMTP_DECODER_EVENT_TRUNCATED_LINE); + break; + } + res = SMTPGetLine(state); + } } if (res.status == 1) return res; /* toclient */ } else { while (res.status == 0) { - if (SMTPProcessReply(state, f, pstate, thread_data) == -1) - SCReturnStruct(APP_LAYER_ERROR); - res = SMTPGetLine(state); + if (!state->discard_till_lf) { + if ((state->current_line_len > 0) && + (SMTPProcessReply(state, f, pstate, thread_data) == -1)) + SCReturnStruct(APP_LAYER_ERROR); + if (state->current_line_delimiter_len == 0 && + state->current_line_len == SMTP_LINE_BUFFER_LIMIT) { + state->discard_till_lf = true; + state->consumed = state->input_len + 1; // For the newly found LF + SMTPSetEvent(state, SMTP_DECODER_EVENT_TRUNCATED_LINE); + break; + } + res = SMTPGetLine(state); + } } if (res.status == 1) return res; diff --git a/src/app-layer-smtp.h b/src/app-layer-smtp.h index 72003be9d0..87a523461a 100644 --- a/src/app-layer-smtp.h +++ b/src/app-layer-smtp.h @@ -115,6 +115,9 @@ typedef struct SMTPState_ { int32_t input_len; uint8_t direction; + /* original length of an input */ + int32_t orig_input_len; + /* --parser details-- */ /** current line extracted by the parser from the call to SMTPGetline() */ const uint8_t *current_line; @@ -123,6 +126,8 @@ typedef struct SMTPState_ { uint8_t current_line_delimiter_len; /* Consumed bytes till current line */ int32_t consumed; + /* If rest of the bytes should be discarded in case of long line w/o LF */ + bool discard_till_lf; /** var to indicate parser state */ uint8_t parser_state;