From: Shivani Bhardwaj Date: Tue, 19 Apr 2022 12:05:06 +0000 (+0530) Subject: smtp: fix indefinite buffering if no LF in line X-Git-Tag: suricata-5.0.9~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=412b77cc8c970fedc3b13bb24ad4af88eb65a631;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 5028 --- diff --git a/src/app-layer-smtp.c b/src/app-layer-smtp.c index bb377b0e78..7d640ff682 100644 --- a/src/app-layer-smtp.c +++ b/src/app-layer-smtp.c @@ -110,6 +110,9 @@ #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 }, { "UNABLE_TO_MATCH_REPLY_WITH_REQUEST", @@ -156,7 +159,8 @@ SCEnumCharMap smtp_decoder_event_table[ ] = { SMTP_DECODER_EVENT_DUPLICATE_FIELDS }, { "UNPARSABLE_CONTENT", SMTP_DECODER_EVENT_UNPARSABLE_CONTENT }, - + { "TRUNCATED_LINE", + SMTP_DECODER_EVENT_TRUNCATED_LINE }, { NULL, -1 }, }; @@ -593,6 +597,12 @@ static int SMTPGetLine(SMTPState *state) uint8_t *lf_idx = memchr(state->input, 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; + return 0; + } /* fragmented lines. Decoder event for special cases. Not all * fragmented lines should be treated as a possible evasion * attempt. With multi payload smtp chunks we can have valid @@ -628,6 +638,12 @@ static int SMTPGetLine(SMTPState *state) return -1; } else { + 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; + return 0; + } state->ts_current_line_lf_seen = 1; if (state->ts_current_line_db == 1) { @@ -695,7 +711,13 @@ static int SMTPGetLine(SMTPState *state) uint8_t *lf_idx = memchr(state->input, 0x0a, state->input_len); if (lf_idx == NULL) { - /* fragmented lines. Decoder event for special cases. Not all + 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; + return 0; + } + /* fragmented lines. Decoder event for special cases. Not all * fragmented lines should be treated as a possible evasion * attempt. With multi payload smtp chunks we can have valid * cases of fragmentation. But within the same segment chunk @@ -730,6 +752,12 @@ static int SMTPGetLine(SMTPState *state) return -1; } else { + 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; + return 0; + } state->tc_current_line_lf_seen = 1; if (state->tc_current_line_db == 1) { @@ -1396,20 +1424,39 @@ static int SMTPParse(int direction, Flow *f, SMTPState *state, state->input = input; state->input_len = input_len; state->direction = direction; + int res = SMTPGetLine(state); /* toserver */ if (direction == 0) { - while (SMTPGetLine(state) >= 0) { - if (SMTPProcessRequest(state, f, pstate) == -1) - SCReturnInt(-1); + while (res == 0) { + if (!state->discard_till_lf) { + if ((state->current_line_len > 0) && (SMTPProcessRequest(state, f, pstate) == -1)) + SCReturnInt(-1); + if (state->current_line_delimiter_len == 0 && + state->current_line_len == SMTP_LINE_BUFFER_LIMIT) { + SMTPSetEvent(state, SMTP_DECODER_EVENT_TRUNCATED_LINE); + state->discard_till_lf = true; + break; + } + res = SMTPGetLine(state); + } } /* toclient */ } else { - while (SMTPGetLine(state) >= 0) { - if (SMTPProcessReply(state, f, pstate, thread_data) == -1) - SCReturnInt(-1); + while (res == 0) { + if (!state->discard_till_lf) { + if ((state->current_line_len > 0) && (SMTPProcessReply(state, f, pstate, thread_data) == -1)) + SCReturnInt(-1); + if (state->current_line_delimiter_len == 0 && + state->current_line_len == SMTP_LINE_BUFFER_LIMIT) { + SMTPSetEvent(state, SMTP_DECODER_EVENT_TRUNCATED_LINE); + state->discard_till_lf = true; + break; + } + res = SMTPGetLine(state); } + } } SCReturnInt(0); diff --git a/src/app-layer-smtp.h b/src/app-layer-smtp.h index 66b28b5e83..8851b88d1f 100644 --- a/src/app-layer-smtp.h +++ b/src/app-layer-smtp.h @@ -55,6 +55,8 @@ enum { /* Invalid behavior or content */ SMTP_DECODER_EVENT_DUPLICATE_FIELDS, SMTP_DECODER_EVENT_UNPARSABLE_CONTENT, + /* For line >= 4KB */ + SMTP_DECODER_EVENT_TRUNCATED_LINE, }; typedef struct SMTPString_ { @@ -124,6 +126,8 @@ typedef struct SMTPState_ { /** length of the line in current_line. Doesn't include the delimiter */ int32_t current_line_len; uint8_t current_line_delimiter_len; + /* If rest of the bytes should be discarded in case of long line w/o LF */ + bool discard_till_lf; /** used to indicate if the current_line buffer is a malloced buffer. We * use a malloced buffer, if a line is fragmented */