Skip to content

Commit 638b1de

Browse files
committed
smtp: complete BDAT transactions at LAST
Track the BDAT LAST marker so the final chunk and its reply complete the transaction in each direction, preventing a following MAIL FROM from being merged into the previous transaction. Ticket: OISF#8741
1 parent 0dcca0c commit 638b1de

1 file changed

Lines changed: 77 additions & 21 deletions

File tree

src/app-layer-smtp.c

Lines changed: 77 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,8 @@
9696
#define SMTP_COMMAND_OTHER_CMD 5
9797
#define SMTP_COMMAND_RSET 6
9898
#define SMTP_COMMAND_QUIT 7
99+
/* Pseudo command used to match the final BDAT reply to its transaction. */
100+
#define SMTP_COMMAND_BDAT_LAST 8
99101

100102
#define SMTP_DEFAULT_MAX_TX 256
101103

@@ -755,7 +757,7 @@ static int SMTPInsertCommandIntoCommandBuffer(
755757
return 0;
756758
}
757759

758-
static int SMTPProcessCommandBDAT(SMTPState *state, const SMTPLine *line)
760+
static int SMTPProcessCommandBDAT(SMTPState *state, SMTPTransaction *tx, const SMTPLine *line)
759761
{
760762
SCEnter();
761763

@@ -767,6 +769,9 @@ static int SMTPProcessCommandBDAT(SMTPState *state, const SMTPLine *line)
767769
SCReturnInt(-1);
768770
} else if (state->bdat_chunk_idx == state->bdat_chunk_len) {
769771
state->parser_state &= ~SMTP_PARSER_STATE_COMMAND_DATA_MODE;
772+
if (state->current_command == SMTP_COMMAND_BDAT_LAST) {
773+
SMTPTransactionCompleteTS(tx);
774+
}
770775
}
771776

772777
SCReturnInt(0);
@@ -1080,6 +1085,10 @@ static int SMTPProcessReply(
10801085
}
10811086
} else if (IsReplyToCommand(state, SMTP_COMMAND_BDAT)) {
10821087
SMTPSetProgressTC(reply_tx, SMTP_RESPONSE_DATA);
1088+
} else if (IsReplyToCommand(state, SMTP_COMMAND_BDAT_LAST)) {
1089+
if (reply_tx && !(state->parser_state & SMTP_PARSER_STATE_PARSING_MULTILINE_REPLY)) {
1090+
SMTPTransactionCompleteTC(reply_tx);
1091+
}
10831092
} else if (IsReplyToCommand(state, SMTP_COMMAND_DATA_MODE)) {
10841093
if (!(state->parser_state & SMTP_PARSER_STATE_PARSING_MULTILINE_REPLY)) {
10851094
SMTPTransactionCompleteTC(reply_tx);
@@ -1121,10 +1130,12 @@ static int SMTPProcessReply(
11211130
return 0;
11221131
}
11231132

1124-
static int SMTPParseCommandBDAT(SMTPState *state, const SMTPLine *line)
1133+
static int SMTPParseCommandBDAT(SMTPState *state, const SMTPLine *line, bool *last)
11251134
{
11261135
SCEnter();
11271136

1137+
*last = false;
1138+
11281139
int i = 4;
11291140
while (i < line->len) {
11301141
if (line->buf[i] != ' ') {
@@ -1148,10 +1159,25 @@ static int SMTPParseCommandBDAT(SMTPState *state, const SMTPLine *line)
11481159
}
11491160
memcpy(strbuf, line->buf + i, len);
11501161
strbuf[len] = '\0';
1151-
if (ByteExtractStringUint32(&state->bdat_chunk_len, 10, 0, strbuf) < 0) {
1162+
int parsed = ByteExtractStringUint32(&state->bdat_chunk_len, 10, 0, strbuf);
1163+
if (parsed < 0) {
11521164
/* decoder event */
11531165
return -1;
11541166
}
1167+
state->bdat_chunk_idx = 0;
1168+
1169+
i += parsed;
1170+
if (i < line->len && line->buf[i] != ' ') {
1171+
return -1;
1172+
}
1173+
while (i < line->len && line->buf[i] == ' ') {
1174+
i++;
1175+
}
1176+
if (line->len - i == 4 && SCMemcmpLowercase("last", line->buf + i, 4) == 0) {
1177+
*last = true;
1178+
} else if (i != line->len) {
1179+
return -1;
1180+
}
11551181

11561182
return 0;
11571183
}
@@ -1364,13 +1390,18 @@ static int SMTPProcessRequest(
13641390
state->parser_state |= SMTP_PARSER_STATE_COMMAND_DATA_MODE;
13651391
}
13661392
} else if (line->len >= 4 && SCMemcmpLowercase("bdat", line->buf, 4) == 0) {
1367-
r = SMTPParseCommandBDAT(state, line);
1393+
bool last = false;
1394+
r = SMTPParseCommandBDAT(state, line, &last);
13681395
if (r == -1) {
13691396
SCReturnInt(-1);
13701397
}
1371-
state->current_command = SMTP_COMMAND_BDAT;
1398+
state->current_command = last ? SMTP_COMMAND_BDAT_LAST : SMTP_COMMAND_BDAT;
13721399
SMTPSetProgressTS(tx, SMTP_REQUEST_DATA);
1373-
state->parser_state |= SMTP_PARSER_STATE_COMMAND_DATA_MODE;
1400+
if (state->bdat_chunk_len > 0) {
1401+
state->parser_state |= SMTP_PARSER_STATE_COMMAND_DATA_MODE;
1402+
} else if (last) {
1403+
SMTPTransactionCompleteTS(tx);
1404+
}
13741405
} else if (line->len >= 4 && ((SCMemcmpLowercase("helo", line->buf, 4) == 0) ||
13751406
SCMemcmpLowercase("ehlo", line->buf, 4) == 0)) {
13761407
r = SMTPParseCommandHELO(state, line);
@@ -1414,7 +1445,8 @@ static int SMTPProcessRequest(
14141445
return SMTPProcessCommandDATA(state, tx, f, line);
14151446

14161447
case SMTP_COMMAND_BDAT:
1417-
return SMTPProcessCommandBDAT(state, line);
1448+
case SMTP_COMMAND_BDAT_LAST:
1449+
return SMTPProcessCommandBDAT(state, tx, line);
14181450

14191451
default:
14201452
/* we have nothing to do with any other command at this instant.
@@ -1432,16 +1464,33 @@ static inline void ResetLine(SMTPLine *line)
14321464
}
14331465
}
14341466

1467+
static int SMTPPreProcessCommandBDAT(
1468+
SMTPState *state, Flow *f, StreamSlice *slice, SMTPInput *input, SMTPLine *line)
1469+
{
1470+
if (state->bdat_chunk_idx >= state->bdat_chunk_len) {
1471+
/* The BDAT chunk is already complete; data mode was set by another
1472+
* command, such as a pipelined DATA reply. Leave data mode and let
1473+
* the line parser handle the input as a new command. */
1474+
state->parser_state &= ~SMTP_PARSER_STATE_COMMAND_DATA_MODE;
1475+
return 1;
1476+
}
1477+
uint32_t remaining = state->bdat_chunk_len - state->bdat_chunk_idx;
1478+
uint32_t consumed = MIN((uint32_t)input->len, remaining);
1479+
line->buf = input->buf + input->consumed;
1480+
line->len = consumed;
1481+
input->consumed += consumed;
1482+
input->len -= consumed;
1483+
int ret = SMTPProcessRequest(state, f, input, line, slice);
1484+
ResetLine(line);
1485+
return ret;
1486+
}
1487+
14351488
/*
1436-
* @brief Pre Process the data that comes in DATA mode.
1489+
* @brief Pre-process command data.
14371490
*
1438-
* If currently, the command that is being processed is DATA, whatever data
1439-
* comes as a part of it must be handled by this function. This is because
1440-
* there should be no char limit imposition on the line arriving in the DATA
1441-
* mode. Such limits are in place for any lines passed to the GetLine function
1442-
* and the lines are capped there at SMTP_LINE_BUFFER_LIMIT.
1443-
* One such limit in DATA mode may lead to file data or parts of e-mail being
1444-
* truncated if the line were too long.
1491+
* BDAT data is octet-counted and must be consumed exactly up to the declared
1492+
* chunk boundary. DATA content is handled here to avoid the line length limit
1493+
* imposed by GetLine, which could otherwise truncate file data or e-mail.
14451494
*
14461495
* @param state Pointer to the current SMTPState
14471496
* @param f Pointer to the current Flow
@@ -1459,6 +1508,11 @@ static int SMTPPreProcessCommands(
14591508
DEBUG_VALIDATE_BUG_ON(line->len != 0);
14601509
DEBUG_VALIDATE_BUG_ON(line->delim_len != 0);
14611510

1511+
if (state->current_command == SMTP_COMMAND_BDAT ||
1512+
state->current_command == SMTP_COMMAND_BDAT_LAST) {
1513+
return SMTPPreProcessCommandBDAT(state, f, slice, input, line);
1514+
}
1515+
14621516
/* fall back to strict line parsing for mime header parsing */
14631517
if (state->curr_tx && state->curr_tx->mime_state &&
14641518
SCMimeSmtpGetState(state->curr_tx->mime_state) < MimeSmtpBody)
@@ -1550,7 +1604,8 @@ static AppLayerResult SMTPParse(uint8_t direction, Flow *f, SMTPState *state,
15501604
/* toserver */
15511605
if (direction == 0) {
15521606
if (((state->current_command == SMTP_COMMAND_DATA) ||
1553-
(state->current_command == SMTP_COMMAND_BDAT)) &&
1607+
(state->current_command == SMTP_COMMAND_BDAT) ||
1608+
(state->current_command == SMTP_COMMAND_BDAT_LAST)) &&
15541609
(state->parser_state & SMTP_PARSER_STATE_COMMAND_DATA_MODE)) {
15551610
int ret = SMTPPreProcessCommands(state, f, &stream_slice, &input, &line);
15561611
DEBUG_VALIDATE_BUG_ON(ret != 0 && ret != -1 && ret != 1);
@@ -1577,11 +1632,12 @@ static AppLayerResult SMTPParse(uint8_t direction, Flow *f, SMTPState *state,
15771632
* wherever it had to be */
15781633
ResetLine(&line);
15791634

1580-
/* If DATA mode was entered in the middle of input parsing, exempt it from GetLine as we
1581-
* don't want input limits to be exercised on DATA data. Here, SMTPPreProcessCommands
1582-
* should either consume all the data or return in case it encounters another boundary.
1583-
* In case of another boundary, the control should be passed to SMTPGetLine */
1584-
if ((input.len > 0) && (state->current_command == SMTP_COMMAND_DATA) &&
1635+
/* If command data mode was entered in the middle of input parsing, exempt it from
1636+
* GetLine so DATA is not truncated and BDAT stops at its exact chunk boundary. */
1637+
if ((input.len > 0) &&
1638+
((state->current_command == SMTP_COMMAND_DATA) ||
1639+
(state->current_command == SMTP_COMMAND_BDAT) ||
1640+
(state->current_command == SMTP_COMMAND_BDAT_LAST)) &&
15851641
(state->parser_state & SMTP_PARSER_STATE_COMMAND_DATA_MODE)) {
15861642
int ret = SMTPPreProcessCommands(state, f, &stream_slice, &input, &line);
15871643
DEBUG_VALIDATE_BUG_ON(ret != 0 && ret != -1 && ret != 1);

0 commit comments

Comments
 (0)