From: Eric Leblond <eric@regit.org>
Date: Tue, 1 Mar 2016 13:59:13 +0000 (+0100)
Subject: app-layer-smtp: fix memory leak
X-Git-Tag: suricata-3.0.1RC1~80
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5dbedbfa5bacf8a02459b2389596b63ec7822aa0;p=thirdparty%2Fsuricata.git

app-layer-smtp: fix memory leak

This patch fixes the following leak:

Direct leak of 9982880 byte(s) in 2902 object(s) allocated from:
    #0 0x4c253b in malloc ??:?
    #1 0x10c39ac in MimeDecInitParser /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/util-decode-mime.c:2379
    #2 0x6a0f91 in SMTPProcessRequest /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/app-layer-smtp.c:1085
    #3 0x697658 in SMTPParse /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/app-layer-smtp.c:1185
    #4 0x68fa7a in SMTPParseClientRecord /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/app-layer-smtp.c:1208
    #5 0x6561c5 in AppLayerParserParse /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/app-layer-parser.c:908
    #6 0x53dc2e in AppLayerHandleTCPData /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/app-layer.c:444
    #7 0xf8e0af in DoReassemble /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/stream-tcp-reassemble.c:2635
    #8 0xf8c3f8 in StreamTcpReassembleAppLayer /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/stream-tcp-reassemble.c:3028
    #9 0xf94267 in StreamTcpReassembleHandleSegmentUpdateACK /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/stream-tcp-reassemble.c:3404
    #10 0xf9643d in StreamTcpReassembleHandleSegment /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/stream-tcp-reassemble.c:3432
    #11 0xf578b4 in HandleEstablishedPacketToClient /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/stream-tcp.c:2245
    #12 0xeea3c7 in StreamTcpPacketStateEstablished /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/stream-tcp.c:2489
    #13 0xec1d38 in StreamTcpPacket /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/stream-tcp.c:4568
    #14 0xeb0e16 in StreamTcp /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/stream-tcp.c:5064
    #15 0xff52a4 in TmThreadsSlotVarRun /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/tm-threads.c:130
    #16 0xffdad1 in TmThreadsSlotVar /home/victor/qa/buildbot/donkey/z600fuzz/Private/src/tm-threads.c:474
    #17 0x7f7cd678d181 in start_thread /build/buildd/eglibc-2.19/nptl/pthread_create.c:312 (discriminator 2)

We come to this case when a SMTP session contains at least 2 mails
and then the ending of the first is not correctly detected. In that
case, switching to a new tx seems a good solution. This way we still
have partial logging.
---

diff --git a/rules/smtp-events.rules b/rules/smtp-events.rules
index 2c318665d1..89c25acbc7 100644
--- a/rules/smtp-events.rules
+++ b/rules/smtp-events.rules
@@ -28,4 +28,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 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;)
 
 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
+alert smtp any any -> any any (msg:"SURICATA SMTP unparsable content"; flow:established,to_server; app-layer-event:smtp.unparsable_content; flowint:smtp.anomaly.count,+,1; classtype:protocol-command-decode; sid:2220019; rev:1;)
+# next sid 2220020
diff --git a/src/app-layer-smtp.c b/src/app-layer-smtp.c
index a753fd8295..c5544de93d 100644
--- a/src/app-layer-smtp.c
+++ b/src/app-layer-smtp.c
@@ -147,6 +147,8 @@ SCEnumCharMap smtp_decoder_event_table[ ] = {
     /* Invalid behavior or content */
     { "DUPLICATE_FIELDS",
       SMTP_DECODER_EVENT_DUPLICATE_FIELDS },
+    { "UNPARSABLE_CONTENT",
+      SMTP_DECODER_EVENT_UNPARSABLE_CONTENT },
 
     { NULL,                      -1 },
 };
@@ -1094,6 +1096,18 @@ static int SMTPProcessRequest(SMTPState *state, Flow *f,
                    SCMemcmpLowercase("data", state->current_line, 4) == 0) {
             state->current_command = SMTP_COMMAND_DATA;
             if (smtp_config.decode_mime) {
+                if (tx->mime_state) {
+                    /* We have 2 chained mails and did not detect the end
+                     * of first one. So we start a new transaction. */
+                    tx->mime_state->state_flag = PARSE_ERROR;
+                    SMTPSetEvent(state, SMTP_DECODER_EVENT_UNPARSABLE_CONTENT);
+                    tx = SMTPTransactionCreate();
+                    if (tx == NULL)
+                        return -1;
+                    state->curr_tx = tx;
+                    TAILQ_INSERT_TAIL(&state->tx_list, tx, next);
+                    tx->tx_id = state->tx_cnt++;
+                }
                 tx->mime_state = MimeDecInitParser(f, SMTPProcessDataChunk);
                 if (tx->mime_state == NULL) {
                     SCLogError(SC_ERR_MEM_ALLOC, "MimeDecInitParser() failed to "
diff --git a/src/app-layer-smtp.h b/src/app-layer-smtp.h
index 7bd252b2ed..06ade068c7 100644
--- a/src/app-layer-smtp.h
+++ b/src/app-layer-smtp.h
@@ -52,6 +52,7 @@ enum {
 
     /* Invalid behavior or content */
     SMTP_DECODER_EVENT_DUPLICATE_FIELDS,
+    SMTP_DECODER_EVENT_UNPARSABLE_CONTENT,
 };
 
 typedef struct SMTPString_ {