From 10e2e2a8b6e5ffabcbc85708c57627dc0be7e087 Mon Sep 17 00:00:00 2001
From: Eric Leblond <eric@regit.org>
Date: Tue, 1 Mar 2016 15:44:27 +0100
Subject: [PATCH] app-layer-smtp: fix mem leak and add new alert

If SMTP session is weird then we may reach a state where a field
like MAIL FROM is seen as duplicated.

Valgrind output is:

30 bytes in 1 blocks are definitely lost in loss record 96 of 399
   at 0x4C29C0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x4A5803: SMTPParseCommandWithParam (app-layer-smtp.c:996)
   by 0x4A4DCE: SMTPParseCommandMAILFROM (app-layer-smtp.c:1016)
   by 0x4A3F55: SMTPProcessRequest (app-layer-smtp.c:1127)
   by 0x4A1F8C: SMTPParse (app-layer-smtp.c:1191)
   by 0x493AD7: SMTPParseClientRecord (app-layer-smtp.c:1214)
   by 0x4878A6: AppLayerParserParse (app-layer-parser.c:908)
   by 0x42384E: AppLayerHandleTCPData (app-layer.c:444)
   by 0x8D7EAD: DoReassemble (stream-tcp-reassemble.c:2635)
   by 0x8D795F: StreamTcpReassembleAppLayer (stream-tcp-reassemble.c:3028)
   by 0x8D8BE0: StreamTcpReassembleHandleSegmentUpdateACK (stream-tcp-reassemble.c:3404)
   by 0x8D8F6E: StreamTcpReassembleHandleSegment (stream-tcp-reassemble.c:3432)
---
 rules/smtp-events.rules |  3 ++-
 src/app-layer-smtp.c    | 12 ++++++++++++
 src/app-layer-smtp.h    |  3 +++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/rules/smtp-events.rules b/rules/smtp-events.rules
index c1ade81ad3..2c318665d1 100644
--- a/rules/smtp-events.rules
+++ b/rules/smtp-events.rules
@@ -27,4 +27,5 @@ alert smtp any any -> any any (msg:"SURICATA SMTP data command rejected"; flow:e
 #alert smtp any any -> any any (msg:"SURICATA SMTP Mime encoded line len exceeded"; flow:established; app-layer-event:smtp.mime_long_enc_line; flowint:smtp.anomaly.count,+,1; classtype:protocol-command-decode; sid:2220016; rev:1;)
 alert smtp any any -> any any (msg:"SURICATA SMTP Mime boundary length exceeded"; flow:established,to_server; app-layer-event:smtp.mime_long_boundary; flowint:smtp.anomaly.count,+,1; classtype:protocol-command-decode; sid:2220017; rev:1;)
 
-# next sid 2220018
+alert smtp any any -> any any (msg:"SURICATA SMTP duplicate fields"; flow:established,to_server; app-layer-event:smtp.duplicate_fields; flowint:smtp.anomaly.count,+,1; classtype:protocol-command-decode; sid:2220018; rev:1;)
+# next sid 2220019
diff --git a/src/app-layer-smtp.c b/src/app-layer-smtp.c
index 8afcf25974..a753fd8295 100644
--- a/src/app-layer-smtp.c
+++ b/src/app-layer-smtp.c
@@ -144,6 +144,10 @@ SCEnumCharMap smtp_decoder_event_table[ ] = {
     { "MIME_LONG_BOUNDARY",
       SMTP_DECODER_EVENT_MIME_BOUNDARY_TOO_LONG },
 
+    /* Invalid behavior or content */
+    { "DUPLICATE_FIELDS",
+      SMTP_DECODER_EVENT_DUPLICATE_FIELDS },
+
     { NULL,                      -1 },
 };
 
@@ -1005,11 +1009,19 @@ static int SMTPParseCommandWithParam(SMTPState *state, uint8_t prefix_len, uint8
 
 static int SMTPParseCommandHELO(SMTPState *state)
 {
+    if (state->helo) {
+        SMTPSetEvent(state, SMTP_DECODER_EVENT_DUPLICATE_FIELDS);
+        return 0;
+    }
     return SMTPParseCommandWithParam(state, 4, &state->helo, &state->helo_len);
 }
 
 static int SMTPParseCommandMAILFROM(SMTPState *state)
 {
+    if (state->curr_tx->mail_from) {
+        SMTPSetEvent(state, SMTP_DECODER_EVENT_DUPLICATE_FIELDS);
+        return 0;
+    }
     return SMTPParseCommandWithParam(state, 9,
                                      &state->curr_tx->mail_from,
                                      &state->curr_tx->mail_from_len);
diff --git a/src/app-layer-smtp.h b/src/app-layer-smtp.h
index c58684143a..7bd252b2ed 100644
--- a/src/app-layer-smtp.h
+++ b/src/app-layer-smtp.h
@@ -49,6 +49,9 @@ enum {
     SMTP_DECODER_EVENT_MIME_LONG_HEADER_NAME,
     SMTP_DECODER_EVENT_MIME_LONG_HEADER_VALUE,
     SMTP_DECODER_EVENT_MIME_BOUNDARY_TOO_LONG,
+
+    /* Invalid behavior or content */
+    SMTP_DECODER_EVENT_DUPLICATE_FIELDS,
 };
 
 typedef struct SMTPString_ {