Skip to content

Commit

Permalink
app-layer-smtp: fix memory leak
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
regit committed Mar 2, 2016
1 parent b6f61f5 commit 4ad1d47
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 1 deletion.
3 changes: 2 additions & 1 deletion rules/smtp-events.rules
Expand Up @@ -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
14 changes: 14 additions & 0 deletions src/app-layer-smtp.c
Expand Up @@ -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 },
};
Expand Down Expand Up @@ -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 "
Expand Down
1 change: 1 addition & 0 deletions src/app-layer-smtp.h
Expand Up @@ -52,6 +52,7 @@ enum {

/* Invalid behavior or content */
SMTP_DECODER_EVENT_DUPLICATE_FIELDS,
SMTP_DECODER_EVENT_UNPARSABLE_CONTENT,
};

typedef struct SMTPString_ {
Expand Down

0 comments on commit 4ad1d47

Please sign in to comment.