Skip to content

Commit

Permalink
app-layer-smtp: fix mem leak and add new alert
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
regit authored and victorjulien committed Mar 2, 2016
1 parent 50ad1ce commit 10e2e2a
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
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 12 additions & 0 deletions src/app-layer-smtp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
};

Expand Down Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions src/app-layer-smtp.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_ {
Expand Down

0 comments on commit 10e2e2a

Please sign in to comment.