smtp/mime: fix null ptr deref on bad traffic

Due to missing error handling, a bad mime message could put the
mime parser in an error state, without the SMTP layer taking this
into account. So the SMTP layer would continue to pass data to the
mime parser, even though it was in an error state.

When the parser would be fed a very long line while in this state,
it would try to set an error flag in the state. However, due to
the error state, this setting of the flag would dereference a null
pointer.

This patch fixes this issue by updating the mime parser to check
the state it is in when receiving new input. It will refuse to
process futher data while in the error state. It will also return
a new error code to indicate to the SMTP layer that the parser
was in an error state.

diff --git a/src/app-layer-smtp.c b/src/app-layer-smtp.c
index b870bdf3e2be01b67f6a12dc98fe1ba17462b307..fc2bb29ffe48d2169fed2f42a9b5ed46cc8bf1dd 100644 (file)
--- a/src/app-layer-smtp.c
+++ b/src/app-layer-smtp.c
@@ -799,6 +799,44 @@ static int SMTPProcessCommandBDAT(SMTPState *state, Flow *f,
     SCReturnInt(0);
 }
 
+static void SetMimeEvents(SMTPState *state)
+{
+    if (state->curr_tx->mime_state->msg == NULL) {
+        return;
+    }
+
+    /* Generate decoder events */
+    MimeDecEntity *msg = state->curr_tx->mime_state->msg;
+    if (msg->anomaly_flags & ANOM_INVALID_BASE64) {
+        SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_INVALID_BASE64);
+    }
+    if (msg->anomaly_flags & ANOM_INVALID_QP) {
+        SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_INVALID_QP);
+    }
+    if (msg->anomaly_flags & ANOM_LONG_LINE) {
+        SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_LONG_LINE);
+    }
+    if (msg->anomaly_flags & ANOM_LONG_ENC_LINE) {
+        SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_LONG_ENC_LINE);
+    }
+    if (msg->anomaly_flags & ANOM_LONG_HEADER_NAME) {
+        SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_LONG_HEADER_NAME);
+    }
+    if (msg->anomaly_flags & ANOM_LONG_HEADER_VALUE) {
+        SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_LONG_HEADER_VALUE);
+    }
+    if (msg->anomaly_flags & ANOM_MALFORMED_MSG) {
+        SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_MALFORMED_MSG);
+    }
+    if (msg->anomaly_flags & ANOM_LONG_BOUNDARY) {
+        SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_BOUNDARY_TOO_LONG);
+    }
+}
+
+/**
+ *  \retval 0 ok
+ *  \retval -1 error
+ */
 static int SMTPProcessCommandDATA(SMTPState *state, Flow *f,
                                   AppLayerParserState *pstate)
 {
@@ -821,37 +859,12 @@ static int SMTPProcessCommandDATA(SMTPState *state, Flow *f,
             /* Complete parsing task */
             int ret = MimeDecParseComplete(state->curr_tx->mime_state);
             if (ret != MIME_DEC_OK) {
-
                 SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_PARSE_FAILED);
                 SCLogDebug("MimeDecParseComplete() function failed");
             }
 
             /* Generate decoder events */
-            MimeDecEntity *msg = state->curr_tx->mime_state->msg;
-            if (msg->anomaly_flags & ANOM_INVALID_BASE64) {
-                SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_INVALID_BASE64);
-            }
-            if (msg->anomaly_flags & ANOM_INVALID_QP) {
-                SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_INVALID_QP);
-            }
-            if (msg->anomaly_flags & ANOM_LONG_LINE) {
-                SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_LONG_LINE);
-            }
-            if (msg->anomaly_flags & ANOM_LONG_ENC_LINE) {
-                SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_LONG_ENC_LINE);
-            }
-            if (msg->anomaly_flags & ANOM_LONG_HEADER_NAME) {
-                SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_LONG_HEADER_NAME);
-            }
-            if (msg->anomaly_flags & ANOM_LONG_HEADER_VALUE) {
-                SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_LONG_HEADER_VALUE);
-            }
-            if (msg->anomaly_flags & ANOM_MALFORMED_MSG) {
-                SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_MALFORMED_MSG);
-            }
-            if (msg->anomaly_flags & ANOM_LONG_BOUNDARY) {
-                SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_BOUNDARY_TOO_LONG);
-            }
+            SetMimeEvents(state);
         }
         state->curr_tx->done = 1;
         SCLogDebug("marked tx as done");
@@ -861,12 +874,20 @@ static int SMTPProcessCommandDATA(SMTPState *state, Flow *f,
     if (state->current_command == SMTP_COMMAND_DATA &&
             (state->parser_state & SMTP_PARSER_STATE_COMMAND_DATA_MODE)) {
 
-        if (smtp_config.decode_mime && state->curr_tx->mime_state) {
+        if (smtp_config.decode_mime && state->curr_tx->mime_state != NULL) {
             int ret = MimeDecParseLine((const uint8_t *) state->current_line,
                     state->current_line_len, state->current_line_delimiter_len,
                     state->curr_tx->mime_state);
             if (ret != MIME_DEC_OK) {
-                SCLogDebug("MimeDecParseLine() function returned an error code: %d", ret);
+                if (ret != MIME_DEC_ERR_STATE) {
+                    /* Generate decoder events */
+                    SetMimeEvents(state);
+
+                    SCLogDebug("MimeDecParseLine() function returned an error code: %d", ret);
+                    SMTPSetEvent(state, SMTP_DECODER_EVENT_MIME_PARSE_FAILED);
+                }
+                /* keep the parser in its error state so we can log that,
+                 * the parser will reject new data */
             }
         }
     }

diff --git a/src/util-decode-mime.c b/src/util-decode-mime.c
index 704533a70cfcafc6a1f8d15ae3a89441c26dd24b..7dbc814d3785cbdde4b8c47111a51c0f0814f5bc 100644 (file)
--- a/src/util-decode-mime.c
+++ b/src/util-decode-mime.c
@@ -2330,6 +2330,11 @@ static int ProcessMimeEntity(const uint8_t *buf, uint32_t len,
 
     SCLogDebug("START FLAG: %s", StateFlags[state->state_flag]);
 
+    if (state->state_flag == PARSE_ERROR) {
+        SCLogDebug("START FLAG: PARSE_ERROR, bail");
+        return MIME_DEC_ERR_STATE;
+    }
+
     /* Track long line */
     if (len > MAX_LINE_LEN) {
         state->stack->top->data->anomaly_flags |= ANOM_LONG_LINE;
@@ -2477,6 +2482,11 @@ int MimeDecParseComplete(MimeDecParseState *state)
 
     SCLogDebug("Parsing flagged as completed");
 
+    if (state->state_flag == PARSE_ERROR) {
+        SCLogDebug("parser in error state: PARSE_ERROR");
+        return MIME_DEC_ERR_STATE;
+    }
+
     /* Store the header value */
     ret = StoreMimeHeader(state);
     if (ret != MIME_DEC_OK) {

diff --git a/src/util-decode-mime.h b/src/util-decode-mime.h
index 0e5fb7cb0516dd8d560935fd4f3c8443e2fc291e..1922264bee387516c2eedb29cd62f51687d45139 100644 (file)
--- a/src/util-decode-mime.h
+++ b/src/util-decode-mime.h
@@ -83,7 +83,8 @@ typedef enum MimeDecRetCode {
     MIME_DEC_MORE = 1,
     MIME_DEC_ERR_DATA = -1,
     MIME_DEC_ERR_MEM = -2,
-    MIME_DEC_ERR_PARSE = -3
+    MIME_DEC_ERR_PARSE = -3,
+    MIME_DEC_ERR_STATE = -4,    /**< parser in error state */
 } MimeDecRetCode;
 
 /**