]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
smtp: fix indefinite buffering if no LF in line
authorShivani Bhardwaj <shivanib134@gmail.com>
Tue, 19 Apr 2022 12:05:06 +0000 (17:35 +0530)
committerVictor Julien <vjulien@oisf.net>
Thu, 21 Apr 2022 05:37:58 +0000 (07:37 +0200)
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

src/app-layer-smtp.c
src/app-layer-smtp.h

index bb377b0e784072b937ef8090dd41dc41c7e27a08..7d640ff682e5242cd9c12bde007663c2b3cdbf1c 100644 (file)
 #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);
index 66b28b5e834f2f31238502d5b5aa63b0ee0f8691..8851b88d1feed6f833e23e83ed95aab11a275158 100644 (file)
@@ -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 */