smtp: treat CR as a line terminator

The ideal line terminator for an SMTP line is <CRLF>. But, given that
bare LF is still allowed by many systems despite the prohibition by
standards, we have to consider that. In order to simplify things, we
consider bare CR as line terminators as well while updating the
delimiter parameter correctly if they were to be followed by a LF
immediately or as a part of next fragment.

This takes care of some edge cases that made base64 decoder error out
because unexpected data was sent to it at times.

Ticket: 5316
(cherry picked from commit 1e3282f36343c59a18d8e342d5e78aa3018b8edd)

diff --git a/src/app-layer-smtp.c b/src/app-layer-smtp.c
index ecdf266ae262478f1cd6927819e1b1ae22a47659..6a9210adb64da9e7360adf94a0267daa0c00cdaf 100644 (file)
--- a/src/app-layer-smtp.c
+++ b/src/app-layer-smtp.c
@@ -40,6 +40,7 @@
 
 #include "util-mpm.h"
 #include "util-debug.h"
+#include "util-print.h"
 #include "util-byte.h"
 #include "util-unittest.h"
 #include "util-unittest-helper.h"
@@ -492,7 +493,6 @@ int SMTPProcessDataChunk(const uint8_t *chunk, uint32_t len,
         } else {
             /* Append data chunk to file */
             SCLogDebug("Appending file...%u bytes", len);
-
             /* 0 is ok, -2 is not stored, -1 is error */
             ret = FileAppendData(files, (uint8_t *) chunk, len);
             if (ret == -2) {
@@ -1040,7 +1040,8 @@ static int SMTPProcessRequest(SMTPState *state, Flow *f,
     SMTPTransaction *tx = state->curr_tx;
 
     /* Line with just LF */
-    if (state->current_line_len == 0 && state->consumed == 1) {
+    if (state->current_line_len == 0 && state->consumed == 1 &&
+            state->current_line_delimiter_len == 0) {
         return 0; // to continue processing further
     }
 
@@ -1203,43 +1204,47 @@ static int SMTPProcessRequest(SMTPState *state, Flow *f,
 
 static int SMTPPreProcessCommands(SMTPState *state, Flow *f, AppLayerParserState *pstate)
 {
-    // By this time we should have had the command line parsed
-    uint8_t *lf_idx = memchr(state->input + state->consumed, 0x0a, state->input_len);
-    const uint32_t orig_input_len = state->input_len;
-    // Both DATA and BDAT set SMTP_PARSER_STATE_COMMAND_DATA_MODE, so this while
-    // loop should be valid for both
-    while (state->input_len > 0 && (state->parser_state & SMTP_PARSER_STATE_COMMAND_DATA_MODE)) {
-        uint8_t delim_len = 0;
-        uint32_t consumed_line = 0;
-        if (lf_idx == NULL) {
+    bool line_complete = false;
+    int32_t input_len = state->input_len;
+    for (int32_t i = 0;
+            i < input_len && (state->parser_state & SMTP_PARSER_STATE_COMMAND_DATA_MODE); i++) {
+        if (state->input[i] == 0x0d) {
+            if (i < input_len - 1 && state->input[i + 1] == 0x0a) {
+                i++;
+                state->current_line_delimiter_len++;
+            }
+            /* Line is just ending in CR */
+            state->current_line_delimiter_len++;
+            line_complete = true;
+        } else if (state->input[i] == 0x0a) {
+            /* Line is just ending in LF */
+            state->current_line_delimiter_len++;
+            line_complete = true;
+        }
+        /* Either line is complete or fragmented */
+        if (line_complete || (i == input_len - 1)) {
+            DEBUG_VALIDATE_BUG_ON(state->consumed + state->input_len != state->orig_input_len);
+            DEBUG_VALIDATE_BUG_ON(state->input_len == 0 && input_len != 0);
+            /* state->input_len reflects data from start of the line in progress. */
             if ((state->input_len == 1 && state->input[state->consumed] == '-') ||
                     (state->input_len > 1 && state->input[state->consumed] == '-' &&
                             state->input[state->consumed + 1] == '-')) {
-                SCLogDebug("possible boundary, yield to getline");
+                SCLogDebug("Possible boundary, yield to GetLine");
                 return 1;
             }
+            int32_t total_consumed = i + 1;
+            int32_t current_line_consumed = total_consumed - state->consumed;
             state->current_line = state->input + state->consumed;
-            state->consumed = state->input_len;
-            consumed_line = state->input_len;
-        } else {
-            state->current_line = state->input + state->consumed;
-            ptrdiff_t idx = lf_idx - state->input;
-            state->consumed = idx + 1;
-            consumed_line = lf_idx - state->current_line + 1;
-            if (orig_input_len >= idx && idx > 0 && state->input[idx - 1] == 0x0d) {
-                delim_len = 2;
-            } else if (orig_input_len == 1 ||
-                       (orig_input_len >= idx && idx > 0 && state->input[idx - 1] != 0x0d)) {
-                delim_len = 1;
+            state->current_line_len = current_line_consumed - state->current_line_delimiter_len;
+            state->consumed = total_consumed;
+            state->input_len -= current_line_consumed;
+            BUG_ON(state->consumed + state->input_len != state->orig_input_len);
+            if (SMTPProcessRequest(state, f, pstate) == -1) {
+                return -1;
             }
+            line_complete = false;
+            state->current_line_delimiter_len = 0;
         }
-        state->current_line_delimiter_len = delim_len;
-        state->current_line_len = consumed_line - delim_len;
-        state->input_len -= (state->current_line_len + delim_len);
-        if (SMTPProcessRequest(state, f, pstate) == -1) {
-            return -1;
-        }
-        lf_idx = memchr(state->input + state->consumed, 0x0a, state->input_len);
     }
     return 0;
 }
@@ -1263,13 +1268,14 @@ static AppLayerResult SMTPParse(int direction, Flow *f, SMTPState *state,
     state->orig_input_len = input_len;
     state->input_len = input_len;
     state->consumed = 0;
+    state->current_line_delimiter_len = 0;
     state->direction = direction;
     if (direction == 0) {
         if (((state->current_command == SMTP_COMMAND_DATA) ||
                     (state->current_command == SMTP_COMMAND_BDAT)) &&
                 (state->parser_state & SMTP_PARSER_STATE_COMMAND_DATA_MODE)) {
             int ret = SMTPPreProcessCommands(state, f, pstate);
-            if (ret == 0 && state->consumed == state->input_len) {
+            if (ret == 0 && state->consumed == state->orig_input_len) {
                 SCReturnStruct(APP_LAYER_OK);
             }
         }
@@ -1281,7 +1287,8 @@ static AppLayerResult SMTPParse(int direction, Flow *f, SMTPState *state,
         while (res.status == 0) {
             BUG_ON(state->discard_till_lf);
             if (!state->discard_till_lf) {
-                if ((state->current_line_len > 0) && (SMTPProcessRequest(state, f, pstate) == -1))
+                if ((state->current_line_delimiter_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) {
@@ -1300,7 +1307,7 @@ static AppLayerResult SMTPParse(int direction, Flow *f, SMTPState *state,
         while (res.status == 0) {
             BUG_ON(state->discard_till_lf);
             if (!state->discard_till_lf) {
-                if ((state->current_line_len > 0) &&
+                if ((state->current_line_delimiter_len > 0) &&
                         (SMTPProcessReply(state, f, pstate, thread_data) == -1))
                     SCReturnStruct(APP_LAYER_ERROR);
                 if (state->current_line_delimiter_len == 0 &&